Namespaced facts fixes (#26615)

* shorten warning on reservd fact collision

also remove ansible_ from namespaced facts for vars manager
handle str conversion errors
use tuple to avoid iterator errors
version added added

* only modify final one

* removed ansible_ removal

lib/ansible/config/data/config.yml

@@ -1241,15 +1241,6 @@ MERGE_MULTIPLE_CLI_TAGS:
   value_type: boolean
   vars: []
   yaml: {key: defaults.merge_multiple_cli_tags}
-NAMESPACE_FACTS:
-  default: False
-  desc: 'TODO: write it'
-  env: [{name: ANSIBLE_RESTRICT_FACTS}]
-  ini:
-  - {key: restrict_facts_namespace, section: defaults}
-  value_type: boolean
-  vars: []
-  yaml: {key: defaults.restrict_facts_namespace}
 NETWORK_GROUP_MODULES:
   default: [eos, nxos, ios, iosxr, junos, ce, vyos, sros, dellos9, dellos10, dellos6]
   desc: 'TODO: write it'
@@ -1259,6 +1250,18 @@ NETWORK_GROUP_MODULES:
   value_type: list
   vars: []
   yaml: {key: defaults.network_group_modules}
+ONLY_NAMESPACE_FACTS:
+  default: False
+  desc:
+    - Facts normally get injected as top level variables, this setting prevents that.
+    - Facts are still available in the `ansible_facts` variable w/o the `ansible_` prefix.
+  env: [{name: ANSIBLE_RESTRICT_FACTS}]
+  ini:
+  - {key: restrict_facts_namespace, section: defaults}
+  value_type: boolean
+  vars: []
+  yaml: {key: defaults.restrict_facts_namespace}
+  version_added: "2.4"
 PARAMIKO_HOST_KEY_AUTO_ADD:
   default: False
   desc: 'TODO: write it'

lib/ansible/executor/task_executor.py

@@ -566,9 +566,7 @@ class TaskExecutor:
                 return failed_when_result
 
             if 'ansible_facts' in result:
-                if not C.NAMESPACE_FACTS:
+                vars_copy.update(result['ansible_facts'])
-                    vars_copy.update(result['ansible_facts'])
-                vars_copy.update({'ansible_facts': result['ansible_facts']})
 
             # set the failed property if it was missing.
             if 'failed' not in result:
@@ -614,9 +612,7 @@ class TaskExecutor:
             variables[self._task.register] = wrap_var(result)
 
         if 'ansible_facts' in result:
-            if not C.NAMESPACE_FACTS:
+            variables.update(result['ansible_facts'])
-                variables.update(result['ansible_facts'])
-            variables.update({'ansible_facts': result['ansible_facts']})
 
         # save the notification target in the result, if it was specified, as
         # this task may be running in a loop in which case the notification

lib/ansible/plugins/action/__init__.py

@@ -812,7 +812,13 @@ class ActionBase(with_metaclass(ABCMeta, object)):
         # then we remove them (except for ssh host keys)
         for r_key in remove_keys:
             if not r_key.startswith('ansible_ssh_host_key_'):
-                display.warning("Removed restricted key from module data: %s = %s" % (r_key, data[r_key]))
+                try:
+                    r_val = to_text(data[r_key])
+                    if len(r_val) > 24:
+                        r_val = '%s ... %s' % (r_val[:13], r_val[-6:])
+                except:
+                    r_val = ' <failed to convert value to a string> '
+                display.warning("Removed restricted key from module data: %s = %s" % (r_key, r_val))
                 del data[r_key]
 
         self._remove_internal_keys(data)

lib/ansible/vars/manager.py

@@ -316,12 +316,14 @@ class VariableManager:
 
             # finally, the facts caches for this host, if it exists
             try:
-                host_facts = wrap_var(self._fact_cache.get(host.name, dict()))
+                host_facts = wrap_var(self._fact_cache.get(host.name, {}))
-                if not C.NAMESPACE_FACTS:
+                if not C.ONLY_NAMESPACE_FACTS:
                     # allow facts to polute main namespace
                     all_vars = combine_vars(all_vars, host_facts)
+
                 # always return namespaced facts
                 all_vars = combine_vars(all_vars, {'ansible_facts': host_facts})
+
             except KeyError:
                 pass