From 1bb33e8e418cbe075644bfe561311289c582be48 Mon Sep 17 00:00:00 2001 From: Will Szumski Date: Tue, 30 Apr 2024 17:16:37 +0100 Subject: [PATCH] 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 --- kayobe/ansible.py | 85 ++++++++++++++----- kayobe/tests/unit/test_ansible.py | 76 +++++++++++------ ...r-internal-playbooks-baed54403608c3e7.yaml | 22 +++++ 3 files changed, 136 insertions(+), 47 deletions(-) create mode 100644 releasenotes/notes/improve-namespacing-for-internal-playbooks-baed54403608c3e7.yaml diff --git a/kayobe/ansible.py b/kayobe/ansible.py index 9dc7cb4db..c1c263858 100644 --- a/kayobe/ansible.py +++ b/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: diff --git a/kayobe/tests/unit/test_ansible.py b/kayobe/tests/unit/test_ansible.py index 40b2437d8..458373f9a 100644 --- a/kayobe/tests/unit/test_ansible.py +++ b/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, diff --git a/releasenotes/notes/improve-namespacing-for-internal-playbooks-baed54403608c3e7.yaml b/releasenotes/notes/improve-namespacing-for-internal-playbooks-baed54403608c3e7.yaml new file mode 100644 index 000000000..e74b2b3d3 --- /dev/null +++ b/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 `__ +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``.