Deprecate defaulting to inject (#85323)

INJECT_FACTS_AS_VARS config deprecate default of true

changelogs/fragments/deprecate_inject.yml

@@ -0,0 +1,3 @@
+deprecated_features:
+- INJECT_FACTS_AS_VARS configuration currently defaults to ``True``, this is now deprecated and it will switch to ``False`` by Ansible 2.24.
+  You will only get notified if you are accessing 'injected' facts (for example, ansible_os_distribution vs ansible_facts['os_distribution']).

lib/ansible/executor/task_executor.py

@@ -47,6 +47,7 @@ display = Display()
 
 
 RETURN_VARS = [x for x in C.MAGIC_VARIABLE_MAPPING.items() if 'become' not in x and '_pass' not in x]
+_INJECT_FACTS, _INJECT_FACTS_ORIGIN = C.config.get_config_value_and_origin('INJECT_FACTS_AS_VARS')
 
 __all__ = ['TaskExecutor']
 
@@ -663,8 +664,11 @@ class TaskExecutor:
                     # TODO: cleaning of facts should eventually become part of taskresults instead of vars
                     af = result['ansible_facts']
                     vars_copy['ansible_facts'] = combine_vars(vars_copy.get('ansible_facts', {}), namespace_facts(af))
-                    if C.INJECT_FACTS_AS_VARS:
-                        cleaned_toplevel = {k: _deprecate_top_level_fact(v) for k, v in clean_facts(af).items()}
+                    if _INJECT_FACTS:
+                        if _INJECT_FACTS_ORIGIN == 'default':
+                            cleaned_toplevel = {k: _deprecate_top_level_fact(v) for k, v in clean_facts(af).items()}
+                        else:
+                            cleaned_toplevel = clean_facts(af)
                         vars_copy.update(cleaned_toplevel)
 
             # set the failed property if it was missing.
@@ -758,9 +762,13 @@ class TaskExecutor:
                 # TODO: cleaning of facts should eventually become part of taskresults instead of vars
                 af = result['ansible_facts']
                 variables['ansible_facts'] = combine_vars(variables.get('ansible_facts', {}), namespace_facts(af))
-                if C.INJECT_FACTS_AS_VARS:
-                    # DTFIX-FUTURE: why is this happening twice, esp since we're post-fork and these will be discarded?
-                    cleaned_toplevel = {k: _deprecate_top_level_fact(v) for k, v in clean_facts(af).items()}
+                if _INJECT_FACTS:
+                    if _INJECT_FACTS_ORIGIN == 'default':
+                        # This happens x2 due to loops and being able to use values in subsequent iterations
+                        # these copies are later discared in favor of 'total/final' one on loop end.
+                        cleaned_toplevel = {k: _deprecate_top_level_fact(v) for k, v in clean_facts(af).items()}
+                    else:
+                        cleaned_toplevel = clean_facts(af)
                     variables.update(cleaned_toplevel)
 
         # save the notification target in the result, if it was specified, as

lib/ansible/vars/manager.py

@@ -50,12 +50,12 @@ if t.TYPE_CHECKING:
 display = Display()
 
 # deprecated: description='enable top-level facts deprecation' core_version='2.20'
-# _DEPRECATE_TOP_LEVEL_FACT_TAG = _tags.Deprecated(
-#     msg='Top-level facts are deprecated.',
-#     version='2.24',
-#     deprecator=_deprecator.ANSIBLE_CORE_DEPRECATOR,
-#     help_text='Use `ansible_facts` instead.',
-# )
+_DEPRECATE_TOP_LEVEL_FACT_TAG = _tags.Deprecated(
+    msg='INJECT_FACTS_AS_VARS default to `True` is deprecated, top-level facts will not be auto injected after the change.',
+    version='2.24',
+    deprecator=_deprecator.ANSIBLE_CORE_DEPRECATOR,
+    help_text='Use `ansible_facts["fact_name"]` (no `ansible_` prefix) instead.',
+)
 
 
 def _deprecate_top_level_fact(value: t.Any) -> t.Any:
@@ -65,8 +65,7 @@ def _deprecate_top_level_fact(value: t.Any) -> t.Any:
     Unique tag instances are required to achieve the correct de-duplication within a top-level templating operation.
     """
     # deprecated: description='enable top-level facts deprecation' core_version='2.20'
-    # return _DEPRECATE_TOP_LEVEL_FACT_TAG.tag(value)
-    return value
+    return _DEPRECATE_TOP_LEVEL_FACT_TAG.tag(value)
 
 
 def preprocess_vars(a):
@@ -284,8 +283,7 @@ class VariableManager:
             all_vars = _combine_and_track(all_vars, _plugins_inventory([host]), "inventory host_vars for '%s'" % host)
             all_vars = _combine_and_track(all_vars, _plugins_play([host]), "playbook host_vars for '%s'" % host)
 
-            # finally, the facts caches for this host, if it exists
-            # TODO: cleaning of facts should eventually become part of taskresults instead of vars
+            # finally, the facts caches for this host, if they exist
             try:
                 try:
                     facts = self._fact_cache.get(host.name)
@@ -294,12 +292,16 @@ class VariableManager:
 
                 all_vars |= namespace_facts(facts)
 
+                inject, origin = C.config.get_config_value_and_origin('INJECT_FACTS_AS_VARS')
                 # push facts to main namespace
-                if C.INJECT_FACTS_AS_VARS:
-                    deprecated_facts_vars = {k: _deprecate_top_level_fact(v) for k, v in clean_facts(facts).items()}
-                    all_vars = _combine_and_track(all_vars, deprecated_facts_vars, "facts")
+                if inject:
+                    if origin == 'default':
+                        clean_top = {k: _deprecate_top_level_fact(v) for k, v in clean_facts(facts).items()}
+                    else:
+                        clean_top = clean_facts(facts)
+                    all_vars = _combine_and_track(all_vars, clean_top, "facts")
                 else:
-                    # always 'promote' ansible_local
+                    # always 'promote' ansible_local, even if empty
                     all_vars = _combine_and_track(all_vars, {'ansible_local': facts.get('ansible_local', {})}, "facts")
             except KeyError:
                 pass

test/integration/targets/deprecations/injectfacts.yml

@@ -0,0 +1,5 @@
+- hosts: localhost
+  gather_facts: true
+  tasks:
+  - debug:
+      msg: '{{ansible_distribution}}'

test/integration/targets/deprecations/runme.sh

@@ -55,3 +55,6 @@ export ANSIBLE_CACHE_PLUGIN=notjsonfile
 
 # check for plugin deprecation
 [ "$(ansible-doc -t cache notjsonfile --playbook-dir ./ | grep -c 'DEPRECATED:')" -eq "1" ]
+
+# Injection default is deprecated
+[ "$(ANSIBLE_INJECT_FACT_VARS=1 ansible-playbook injectfacts.yml 2>&1 | grep -c 'INJECT_FACTS_AS_VARS')" -eq "0" ]