Prevent accidental overriding of Ansible extensions

When using the custom playbook feature, it is possible to affect the
behaviour of internal kayobe playbooks by installing newer versions of
roles, collections, or plugins. This is almost always undesirable. It
occurs because ansible extensions in kayobe config currently have
precedence over the kayobe internal variants. We can prevent users
accidentally breaking kayobe internal playbooks by searching for
extensions in paths in the kayobe install first, followed by kayobe
config (but only when running internal playbooks). The behaviour when
running external playbooks is unchanged. This method still allows you to
install additional plugins, which can be useful in kayobe config e.g
processing a variable with a custom filter plugin.

Change-Id: I34f0351dbcb50104c9a4d6706d94a349c3ea3b9f
Closes-Bug: #2056473
Co-Authored-By: Matt Crees <mattc@stackhpc.com>

kayobe/ansible.py

@@ -21,7 +21,6 @@ import subprocess
 import sys
 import tempfile
 
-import ansible.constants
 from ansible.parsing.yaml.objects import AnsibleVaultEncryptedUnicode
 
 from kayobe import exception
@@ -220,7 +219,7 @@ def build_args(parsed_args, playbooks,
     return cmd
 
 
-def _get_environment(parsed_args):
+def _get_environment(parsed_args, external_playbook=False):
     """Return an environment dict for executing an Ansible playbook."""
     env = os.environ.copy()
     vault.update_environment(parsed_args, env)
@@ -240,34 +239,69 @@ def _get_environment(parsed_args):
     # Update various role, collection and plugin paths to include the Kayobe
     # roles, collections and plugins. This allows custom playbooks to use these
     # resources.
-    roles_paths = [
-        os.path.join(parsed_args.config_path, "ansible", "roles"),
-        utils.get_data_files_path("ansible", "roles"),
-    ] + ansible.constants.DEFAULT_ROLES_PATH
+    if external_playbook:
+        roles_paths = [
+            os.path.join(parsed_args.config_path, "ansible", "roles"),
+            utils.get_data_files_path("ansible", "roles"),
+        ]
+    else:
+        roles_paths = [
+            utils.get_data_files_path("ansible", "roles"),
+            os.path.join(parsed_args.config_path, "ansible", "roles"),
+        ]
+
     env.setdefault("ANSIBLE_ROLES_PATH", ":".join(roles_paths))
 
-    collections_paths = [
-        os.path.join(parsed_args.config_path, "ansible", "collections"),
-        utils.get_data_files_path("ansible", "collections"),
-    ] + ansible.constants.COLLECTIONS_PATHS
+    if external_playbook:
+        collections_paths = [
+            os.path.join(parsed_args.config_path, "ansible", "collections"),
+            utils.get_data_files_path("ansible", "collections"),
+        ]
+    else:
+        collections_paths = [
+            utils.get_data_files_path("ansible", "collections"),
+            os.path.join(parsed_args.config_path, "ansible", "collections"),
+        ]
+
     env.setdefault("ANSIBLE_COLLECTIONS_PATH", ":".join(collections_paths))
 
-    action_plugins = [
-        os.path.join(parsed_args.config_path, "ansible", "action_plugins"),
-        utils.get_data_files_path("ansible", "action_plugins"),
-    ] + ansible.constants.DEFAULT_ACTION_PLUGIN_PATH
+    if external_playbook:
+        action_plugins = [
+            os.path.join(parsed_args.config_path, "ansible", "action_plugins"),
+            utils.get_data_files_path("ansible", "action_plugins"),
+        ]
+    else:
+        action_plugins = [
+            utils.get_data_files_path("ansible", "action_plugins"),
+            os.path.join(parsed_args.config_path, "ansible", "action_plugins"),
+        ]
+
     env.setdefault("ANSIBLE_ACTION_PLUGINS", ":".join(action_plugins))
 
-    filter_plugins = [
-        os.path.join(parsed_args.config_path, "ansible", "filter_plugins"),
-        utils.get_data_files_path("ansible", "filter_plugins"),
-    ] + ansible.constants.DEFAULT_FILTER_PLUGIN_PATH
+    if external_playbook:
+        filter_plugins = [
+            os.path.join(parsed_args.config_path, "ansible", "filter_plugins"),
+            utils.get_data_files_path("ansible", "filter_plugins"),
+        ]
+    else:
+        filter_plugins = [
+            utils.get_data_files_path("ansible", "filter_plugins"),
+            os.path.join(parsed_args.config_path, "ansible", "filter_plugins"),
+        ]
+
     env.setdefault("ANSIBLE_FILTER_PLUGINS", ":".join(filter_plugins))
 
-    test_plugins = [
-        os.path.join(parsed_args.config_path, "ansible", "test_plugins"),
-        utils.get_data_files_path("ansible", "test_plugins"),
-    ] + ansible.constants.DEFAULT_TEST_PLUGIN_PATH
+    if external_playbook:
+        test_plugins = [
+            os.path.join(parsed_args.config_path, "ansible", "test_plugins"),
+            utils.get_data_files_path("ansible", "test_plugins"),
+        ]
+    else:
+        test_plugins = [
+            utils.get_data_files_path("ansible", "test_plugins"),
+            os.path.join(parsed_args.config_path, "ansible", "test_plugins"),
+        ]
+
     env.setdefault("ANSIBLE_TEST_PLUGINS", ":".join(test_plugins))
 
     return env
@@ -284,7 +318,12 @@ def run_playbooks(parsed_args, playbooks,
                      verbose_level=verbose_level, check=check,
                      ignore_limit=ignore_limit, list_tasks=list_tasks,
                      diff=diff)
-    env = _get_environment(parsed_args)
+    first_playbook = os.path.realpath(playbooks[0])
+    external_playbook = False
+    if not first_playbook.startswith(os.path.realpath(
+            utils.get_data_files_path("ansible"))):
+        external_playbook = True
+    env = _get_environment(parsed_args, external_playbook)
     try:
         utils.run_command(cmd, check_output=check_output, quiet=quiet, env=env)
     except subprocess.CalledProcessError as e:

kayobe/tests/unit/test_ansible.py

@@ -54,39 +54,78 @@ class TestCase(unittest.TestCase):
             "playbook1.yml",
             "playbook2.yml",
         ]
-        home = os.path.expanduser("~")
+
         expected_env = {
             "KAYOBE_CONFIG_PATH": "/etc/kayobe",
             "ANSIBLE_ROLES_PATH": ":".join([
                 "/etc/kayobe/ansible/roles",
                 utils.get_data_files_path("ansible", "roles"),
-                home + "/.ansible/roles",
-                "/usr/share/ansible/roles",
-                "/etc/ansible/roles",
             ]),
             "ANSIBLE_COLLECTIONS_PATH": ":".join([
                 "/etc/kayobe/ansible/collections",
                 utils.get_data_files_path("ansible", "collections"),
-                home + "/.ansible/collections",
-                "/usr/share/ansible/collections",
             ]),
             "ANSIBLE_ACTION_PLUGINS": ":".join([
                 "/etc/kayobe/ansible/action_plugins",
                 utils.get_data_files_path("ansible", "action_plugins"),
-                home + "/.ansible/plugins/action",
-                "/usr/share/ansible/plugins/action",
             ]),
             "ANSIBLE_FILTER_PLUGINS": ":".join([
                 "/etc/kayobe/ansible/filter_plugins",
                 utils.get_data_files_path("ansible", "filter_plugins"),
-                home + "/.ansible/plugins/filter",
-                "/usr/share/ansible/plugins/filter",
             ]),
             "ANSIBLE_TEST_PLUGINS": ":".join([
                 "/etc/kayobe/ansible/test_plugins",
                 utils.get_data_files_path("ansible", "test_plugins"),
-                home + "/.ansible/plugins/test",
-                "/usr/share/ansible/plugins/test",
+            ]),
+        }
+        mock_run.assert_called_once_with(expected_cmd, check_output=False,
+                                         quiet=False, env=expected_env)
+        mock_vars.assert_called_once_with(["/etc/kayobe"])
+
+    @mock.patch.object(utils, "run_command")
+    @mock.patch.object(ansible, "_get_vars_files")
+    @mock.patch.object(ansible, "_validate_args")
+    def test_run_playbooks_internal(self, mock_validate, mock_vars, mock_run):
+        mock_vars.return_value = ["/etc/kayobe/vars-file1.yml",
+                                  "/etc/kayobe/vars-file2.yaml"]
+        parser = argparse.ArgumentParser()
+        ansible.add_args(parser)
+        vault.add_args(parser)
+        parsed_args = parser.parse_args([])
+        pb1 = utils.get_data_files_path("ansible", "playbook1.yml")
+        pb2 = utils.get_data_files_path("ansible", "playbook2.yml")
+        ansible.run_playbooks(parsed_args, [pb1, pb2])
+        expected_cmd = [
+            "ansible-playbook",
+            "--inventory", utils.get_data_files_path("ansible", "inventory"),
+            "--inventory", "/etc/kayobe/inventory",
+            "-e", "@/etc/kayobe/vars-file1.yml",
+            "-e", "@/etc/kayobe/vars-file2.yaml",
+            f"{pb1}",
+            f"{pb2}",
+        ]
+
+        expected_env = {
+            "KAYOBE_CONFIG_PATH": "/etc/kayobe",
+            "ANSIBLE_ROLES_PATH": ":".join([
+                utils.get_data_files_path("ansible", "roles"),
+                "/etc/kayobe/ansible/roles",
+            ]),
+            "ANSIBLE_COLLECTIONS_PATH": ":".join([
+                utils.get_data_files_path("ansible", "collections"),
+                "/etc/kayobe/ansible/collections",
+            ]),
+            "ANSIBLE_ACTION_PLUGINS": ":".join([
+                utils.get_data_files_path("ansible", "action_plugins"),
+                "/etc/kayobe/ansible/action_plugins",
+            ]),
+            "ANSIBLE_FILTER_PLUGINS": ":".join([
+                utils.get_data_files_path("ansible", "filter_plugins"),
+                "/etc/kayobe/ansible/filter_plugins",
+            ]),
+            "ANSIBLE_TEST_PLUGINS": ":".join([
+                utils.get_data_files_path("ansible", "test_plugins"),
+                "/etc/kayobe/ansible/test_plugins",
             ]),
         }
         mock_run.assert_called_once_with(expected_cmd, check_output=False,
@@ -182,40 +221,29 @@ class TestCase(unittest.TestCase):
             "playbook1.yml",
             "playbook2.yml",
         ]
-        home = os.path.expanduser("~")
+
         expected_env = {
             "KAYOBE_CONFIG_PATH": "/path/to/config",
             "KAYOBE_ENVIRONMENT": "test-env",
             "ANSIBLE_ROLES_PATH": ":".join([
                 "/path/to/config/ansible/roles",
                 utils.get_data_files_path("ansible", "roles"),
-                home + "/.ansible/roles",
-                "/usr/share/ansible/roles",
-                "/etc/ansible/roles",
             ]),
             "ANSIBLE_COLLECTIONS_PATH": ":".join([
                 "/path/to/config/ansible/collections",
                 utils.get_data_files_path("ansible", "collections"),
-                home + "/.ansible/collections",
-                "/usr/share/ansible/collections",
             ]),
             "ANSIBLE_ACTION_PLUGINS": ":".join([
                 "/path/to/config/ansible/action_plugins",
                 utils.get_data_files_path("ansible", "action_plugins"),
-                home + "/.ansible/plugins/action",
-                "/usr/share/ansible/plugins/action",
             ]),
             "ANSIBLE_FILTER_PLUGINS": ":".join([
                 "/path/to/config/ansible/filter_plugins",
                 utils.get_data_files_path("ansible", "filter_plugins"),
-                home + "/.ansible/plugins/filter",
-                "/usr/share/ansible/plugins/filter",
             ]),
             "ANSIBLE_TEST_PLUGINS": ":".join([
                 "/path/to/config/ansible/test_plugins",
                 utils.get_data_files_path("ansible", "test_plugins"),
-                home + "/.ansible/plugins/test",
-                "/usr/share/ansible/plugins/test",
             ]),
         }
         mock_run.assert_called_once_with(expected_cmd, check_output=False,

releasenotes/notes/improve-namespacing-for-internal-playbooks-baed54403608c3e7.yaml

@@ -0,0 +1,22 @@
+---
+fixes:
+  - |
+    The Ansible search paths, when running Kayobe internal playbooks, have been
+    modified so that collections, roles and plugins internal to the Kayobe
+    installation have precedence over those installed in Kayobe configuration.
+    This improves the usability as it is now possible to install a newer
+    version of an extension without affecting internal Kayobe playbooks.
+    `LP#2056473 <https://launchpad.net/bugs/2056473>`__
+upgrade:
+  - |
+    Ansible plugins, roles, and collections (collectively known as extensions)
+    installed in Kayobe configuration no longer have precedence over internal
+    Kayobe variants of the same extension. You can revert back to the previous
+    behaviour by manually exporting the relevant Ansible variables, e.g
+    ``ANSIBLE_COLLECTIONS_PATH``. It is not anticipated that this will affect
+    many users as it is still possible to supplement Kayobe with additional
+    plugins.
+  - |
+    System folders and home directories are no longer searched when looking for
+    Ansible extensions. It is recommended to install your collections using
+    ``$KAYOBE_CONFIG_PATH/ansible/requirements.yml``.