Fix fact deps when 'filter=ansible_fact' is used. (#33441)

The accumulated collected_facts was being update
with new facts _after_ filtering them. So only
facts that pass the filter would ever be passed
to other fact collectors.

For 'filter=ansible_service_mgr', even though it requires
the platform and distribution facts and even collects them,
they would get filtered out and never passed to the other
collectors that need them (service_mgr for ex).

Fix is just to add the unfiltered facts to collected_facts.

Adds unit tests for fact filter and collected_facts.

Fixes #32286

lib/ansible/module_utils/facts/ansible_collector.py

@@ -67,10 +67,6 @@ class AnsibleFactCollector(collector.BaseFactCollector):
         for collector_obj in self.collectors:
             info_dict = {}
 
-            # shallow copy of the accumulated collected facts to pass to each collector
-            # for reference.
-            collected_facts.update(facts_dict.copy())
-
             try:
 
                 # Note: this collects with namespaces, so collected_facts also includes namespaces
@@ -80,6 +76,10 @@ class AnsibleFactCollector(collector.BaseFactCollector):
                 sys.stderr.write(repr(e))
                 sys.stderr.write('\n')
 
+            # shallow copy of the new facts to pass to each collector in collected_facts so facts
+            # can reference other facts they depend on.
+            collected_facts.update(info_dict.copy())
+
             # NOTE: If we want complicated fact dict merging, this is where it would hook in
             facts_dict.update(self._filter(info_dict, self.filter_spec))
 

test/units/module_utils/facts/test_ansible_collector.py

@@ -75,13 +75,16 @@ ALL_COLLECTOR_CLASSES = \
      FacterFactCollector]
 
 
-def mock_module(gather_subset=None):
+def mock_module(gather_subset=None,
+                filter=None):
     if gather_subset is None:
         gather_subset = ['all', '!facter', '!ohai']
+    if filter is None:
+        filter = '*'
     mock_module = Mock()
     mock_module.params = {'gather_subset': gather_subset,
                           'gather_timeout': 5,
-                          'filter': '*'}
+                          'filter': filter}
     mock_module.get_bin_path = Mock(return_value=None)
     return mock_module
 
@@ -258,6 +261,153 @@ class TestCollectedFacts(unittest.TestCase):
             self.assertNotIn(not_expected_fact, facts_keys)
 
 
+class ProvidesOtherFactCollector(collector.BaseFactCollector):
+    name = 'provides_something'
+    _fact_ids = set(['needed_fact'])
+
+    def collect(self, module=None, collected_facts=None):
+        return {'needed_fact': 'THE_NEEDED_FACT_VALUE'}
+
+
+class RequiresOtherFactCollector(collector.BaseFactCollector):
+    name = 'requires_something'
+
+    def collect(self, module=None, collected_facts=None):
+        collected_facts = collected_facts or {}
+        fact_dict = {}
+        fact_dict['needed_fact'] = collected_facts['needed_fact']
+        fact_dict['compound_fact'] = "compound-%s" % collected_facts['needed_fact']
+        return fact_dict
+
+
+class ConCatFactCollector(collector.BaseFactCollector):
+    name = 'concat_collected'
+
+    def collect(self, module=None, collected_facts=None):
+        collected_facts = collected_facts or {}
+        fact_dict = {}
+        con_cat_list = []
+        for key, value in collected_facts.items():
+            con_cat_list.append(value)
+
+        fact_dict['concat_fact'] = '-'.join(con_cat_list)
+        return fact_dict
+
+
+class TestCollectorDepsWithFilter(unittest.TestCase):
+    gather_subset = ['all', '!facter', '!ohai']
+
+    def _mock_module(self, gather_subset=None, filter=None):
+        return mock_module(gather_subset=self.gather_subset,
+                           filter=filter)
+
+    def setUp(self):
+        self.mock_module = self._mock_module()
+        self.collectors = self._collectors(mock_module)
+
+    def _collectors(self, module,
+                    all_collector_classes=None,
+                    minimal_gather_subset=None):
+        return [ProvidesOtherFactCollector(),
+                RequiresOtherFactCollector()]
+
+    def test_no_filter(self):
+        _mock_module = mock_module(gather_subset=['all', '!facter', '!ohai'])
+
+        facts_dict = self._collect(_mock_module)
+
+        expected = {'needed_fact': 'THE_NEEDED_FACT_VALUE',
+                    'compound_fact': 'compound-THE_NEEDED_FACT_VALUE'}
+
+        self.assertEquals(expected, facts_dict)
+
+    def test_with_filter_on_compound_fact(self):
+        _mock_module = mock_module(gather_subset=['all', '!facter', '!ohai'],
+                                   filter='compound_fact')
+
+        facts_dict = self._collect(_mock_module)
+
+        expected = {'compound_fact': 'compound-THE_NEEDED_FACT_VALUE'}
+
+        self.assertEquals(expected, facts_dict)
+
+    def test_with_filter_on_needed_fact(self):
+        _mock_module = mock_module(gather_subset=['all', '!facter', '!ohai'],
+                                   filter='needed_fact')
+
+        facts_dict = self._collect(_mock_module)
+
+        expected = {'needed_fact': 'THE_NEEDED_FACT_VALUE'}
+
+        self.assertEquals(expected, facts_dict)
+
+    def test_with_filter_on_compound_gather_compound(self):
+        _mock_module = mock_module(gather_subset=['!all', '!any', 'compound_fact'],
+                                   filter='compound_fact')
+
+        facts_dict = self._collect(_mock_module)
+
+        expected = {'compound_fact': 'compound-THE_NEEDED_FACT_VALUE'}
+
+        self.assertEquals(expected, facts_dict)
+
+    def test_with_filter_no_match(self):
+        _mock_module = mock_module(gather_subset=['all', '!facter', '!ohai'],
+                                   filter='ansible_this_doesnt_exist')
+
+        facts_dict = self._collect(_mock_module)
+
+        expected = {}
+        self.assertEquals(expected, facts_dict)
+
+    def test_concat_collector(self):
+        _mock_module = mock_module(gather_subset=['all', '!facter', '!ohai'])
+
+        _collectors = self._collectors(_mock_module)
+        _collectors.append(ConCatFactCollector())
+
+        fact_collector = \
+            ansible_collector.AnsibleFactCollector(collectors=_collectors,
+                                                   namespace=ns,
+                                                   filter_spec=_mock_module.params['filter'])
+
+        collected_facts = {}
+        facts_dict = fact_collector.collect(module=_mock_module,
+                                            collected_facts=collected_facts)
+        self.assertIn('concat_fact', facts_dict)
+        self.assertTrue('THE_NEEDED_FACT_VALUE' in facts_dict['concat_fact'])
+
+    def test_concat_collector_with_filter_on_concat(self):
+        _mock_module = mock_module(gather_subset=['all', '!facter', '!ohai'],
+                                   filter='concat_fact')
+
+        _collectors = self._collectors(_mock_module)
+        _collectors.append(ConCatFactCollector())
+
+        fact_collector = \
+            ansible_collector.AnsibleFactCollector(collectors=_collectors,
+                                                   namespace=ns,
+                                                   filter_spec=_mock_module.params['filter'])
+
+        collected_facts = {}
+        facts_dict = fact_collector.collect(module=_mock_module,
+                                            collected_facts=collected_facts)
+        self.assertIn('concat_fact', facts_dict)
+        self.assertTrue('THE_NEEDED_FACT_VALUE' in facts_dict['concat_fact'])
+        self.assertTrue('compound' in facts_dict['concat_fact'])
+
+    def _collect(self, _mock_module, collected_facts=None):
+        _collectors = self._collectors(_mock_module)
+
+        fact_collector = \
+            ansible_collector.AnsibleFactCollector(collectors=_collectors,
+                                                   namespace=ns,
+                                                   filter_spec=_mock_module.params['filter'])
+        facts_dict = fact_collector.collect(module=_mock_module,
+                                            collected_facts=collected_facts)
+        return facts_dict
+
+
 class ExceptionThrowingCollector(collector.BaseFactCollector):
     def collect(self, module=None, collected_facts=None):
         raise Exception('A collector failed')