Don't cache duplicate plugin names (#34420)

* Don't cache duplicate plugin names. Fixes #33484

* Add tests for duplicate plugin filenames, to showcase what happens

lib/ansible/plugins/loader.py

@@ -424,7 +424,14 @@ class PluginLoader:
                 continue
 
             if path not in self._module_cache:
-                self._module_cache[path] = self._load_module_source(name, path)
+                module = self._load_module_source(name, path)
+                if module in self._module_cache.values():
+                    # In ``_load_module_source`` if a plugin has a duplicate name, we just return the
+                    # previously matched plugin from sys.modules, which means you are never getting both,
+                    # just one, but cached for both paths, this isn't normally a problem, except with callbacks
+                    # where it will run that single callback twice. This rejects duplicates.
+                    continue
+                self._module_cache[path] = module
                 found_in_cache = False
 
             try:

test/units/plugins/loader_fixtures/import_fixture.py

@@ -0,0 +1,8 @@
+# Nothing to see here, this file is just empty to support a imp.load_source
+# without doing anything
+__metaclass__ = type
+
+
+class test:
+    def __init__(self, *args, **kwargs):
+        pass

test/units/plugins/test_plugins.py

@@ -91,3 +91,48 @@ class TestErrors(unittest.TestCase):
 
     def test_plugin__init_config_none(self):
         self.assertPluginLoaderConfigBecomes(None, [])
+
+    def test__load_module_source_no_duplicate_names(self):
+        '''
+        This test simulates importing 2 plugins with the same name,
+        and validating that the import is shortcirtuited if a file with the same name
+        has already been imported
+        '''
+
+        fixture_path = os.path.join(os.path.dirname(__file__), 'loader_fixtures')
+
+        pl = PluginLoader('test', '', 'test', 'test_plugin')
+        one = pl._load_module_source('import_fixture', os.path.join(fixture_path, 'import_fixture.py'))
+        # This line wouldn't even succeed if we didn't short cirtuit on finding a duplicate name
+        two = pl._load_module_source('import_fixture', '/path/to/import_fixture.py')
+
+        self.assertEqual(one, two)
+
+    @patch('ansible.plugins.loader.glob')
+    @patch.object(PluginLoader, '_get_paths')
+    def test_all_no_duplicate_names(self, gp_mock, glob_mock):
+        '''
+        This test goes along with ``test__load_module_source_no_duplicate_names``
+        and ensures that we ignore duplicate imports on multiple paths
+        '''
+
+        fixture_path = os.path.join(os.path.dirname(__file__), 'loader_fixtures')
+
+        gp_mock.return_value = [
+            fixture_path,
+            '/path/to'
+        ]
+
+        glob_mock.glob.side_effect = [
+            [os.path.join(fixture_path, 'import_fixture.py')],
+            ['/path/to/import_fixture.py']
+        ]
+
+        pl = PluginLoader('test', '', 'test', 'test_plugin')
+        # Aside from needing ``list()`` so we can do a len, ``PluginLoader.all`` returns a generator
+        # so ``list()`` actually causes ``PluginLoader.all`` to run.
+        plugins = list(pl.all())
+        self.assertEqual(len(plugins), 1)
+
+        self.assertIn(os.path.join(fixture_path, 'import_fixture.py'), pl._module_cache)
+        self.assertNotIn('/path/to/import_fixture.py', pl._module_cache)