From 78702d0e3094e6d6a16a31eaf2517d4e0f25d1c7 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Tue, 15 Aug 2023 11:19:41 +0100 Subject: [PATCH] Fix configuration dump with inline encrypted variables If inline Ansible vault encryption is used to define an encrypted variable in kayobe-config, running 'kayobe configuration dump -l ' fails with the following: Failed to decode config dump YAML file /tmp/tmp_fg1bv_j/localhost.yml: ConstructorError(None, None, "could not determine a constructor for the tag '!vault'", ) This change fixes the error by using the Ansible YAML loader which supports the vault tag. Any vault encrypted variables are sanitised in the dump output. Note that variables in vault encrypted files are not sanitised. Change-Id: I4830500d3c927b0689b6f0bca32c28137916420b Closes-Bug: #2031390 --- kayobe/ansible.py | 18 ++++- kayobe/tests/unit/test_ansible.py | 66 ++++++++++++++++++- kayobe/tests/unit/test_utils.py | 54 +++++++++++++++ kayobe/utils.py | 20 +++++- .../config-dump-vault-edc615e475f234ac.yaml | 7 ++ 5 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/config-dump-vault-edc615e475f234ac.yaml diff --git a/kayobe/ansible.py b/kayobe/ansible.py index fed57ea4f..9dc7cb4db 100644 --- a/kayobe/ansible.py +++ b/kayobe/ansible.py @@ -22,6 +22,7 @@ import sys import tempfile import ansible.constants +from ansible.parsing.yaml.objects import AnsibleVaultEncryptedUnicode from kayobe import exception from kayobe import utils @@ -299,6 +300,18 @@ def run_playbook(parsed_args, playbook, *args, **kwargs): return run_playbooks(parsed_args, [playbook], *args, **kwargs) +def _sanitise_hostvar(var): + """Sanitise a host variable.""" + if isinstance(var, AnsibleVaultEncryptedUnicode): + return "******" + # Recursively sanitise dicts and lists. + if isinstance(var, dict): + return {k: _sanitise_hostvar(v) for k, v in var.items()} + if isinstance(var, list): + return [_sanitise_hostvar(v) for v in var] + return var + + def config_dump(parsed_args, host=None, hosts=None, var_name=None, facts=None, extra_vars=None, tags=None, verbose_level=None): dump_dir = tempfile.mkdtemp() @@ -324,7 +337,8 @@ def config_dump(parsed_args, host=None, hosts=None, var_name=None, LOG.debug("Found dump file %s", path) inventory_hostname, ext = os.path.splitext(path) if ext == ".yml": - hvars = utils.read_yaml_file(os.path.join(dump_dir, path)) + dump_file = os.path.join(dump_dir, path) + hvars = utils.read_config_dump_yaml_file(dump_file) if host: return hvars else: @@ -332,7 +346,7 @@ def config_dump(parsed_args, host=None, hosts=None, var_name=None, else: LOG.warning("Unexpected extension on config dump file %s", path) - return hostvars + return {k: _sanitise_hostvar(v) for k, v in hostvars.items()} finally: shutil.rmtree(dump_dir) diff --git a/kayobe/tests/unit/test_ansible.py b/kayobe/tests/unit/test_ansible.py index 875688417..40b2437d8 100644 --- a/kayobe/tests/unit/test_ansible.py +++ b/kayobe/tests/unit/test_ansible.py @@ -583,7 +583,7 @@ class TestCase(unittest.TestCase): ansible.run_playbooks, parsed_args, ["command"]) @mock.patch.object(shutil, 'rmtree') - @mock.patch.object(utils, 'read_yaml_file') + @mock.patch.object(utils, 'read_config_dump_yaml_file') @mock.patch.object(os, 'listdir') @mock.patch.object(ansible, 'run_playbook') @mock.patch.object(tempfile, 'mkdtemp') @@ -621,6 +621,70 @@ class TestCase(unittest.TestCase): mock.call(os.path.join(dump_dir, "host2.yml")), ]) + @mock.patch.object(shutil, 'rmtree') + @mock.patch.object(utils, 'read_file') + @mock.patch.object(os, 'listdir') + @mock.patch.object(ansible, 'run_playbook') + @mock.patch.object(tempfile, 'mkdtemp') + def test_config_dump_vaulted(self, mock_mkdtemp, mock_run, mock_listdir, + mock_read, mock_rmtree): + parser = argparse.ArgumentParser() + parsed_args = parser.parse_args([]) + dump_dir = "/path/to/dump" + mock_mkdtemp.return_value = dump_dir + mock_listdir.return_value = ["host1.yml", "host2.yml"] + config = """--- +key1: !vault | + $ANSIBLE_VAULT;1.1;AES256 + 633230623736383232323862393364323037343430393530316636363961626361393133646437 + 643438663261356433656365646138666133383032376532310a63323432306431303437623637 + 346236316161343635636230613838316566383933313338636237616338326439616536316639 + 6334343462333062363334300a3930313762313463613537626531313230303731343365643766 + 666436333037 +key2: value2 +key3: + - !vault | + $ANSIBLE_VAULT;1.1;AES256 + 633230623736383232323862393364323037343430393530316636363961626361393133646437 + 643438663261356433656365646138666133383032376532310a63323432306431303437623637 + 346236316161343635636230613838316566383933313338636237616338326439616536316639 + 6334343462333062363334300a3930313762313463613537626531313230303731343365643766 + 666436333037 +""" + config_nested = """--- +key1: + key2: !vault | + $ANSIBLE_VAULT;1.1;AES256 + 633230623736383232323862393364323037343430393530316636363961626361393133646437 + 643438663261356433656365646138666133383032376532310a63323432306431303437623637 + 346236316161343635636230613838316566383933313338636237616338326439616536316639 + 6334343462333062363334300a3930313762313463613537626531313230303731343365643766 + 666436333037 +""" + mock_read.side_effect = [config, config_nested] + result = ansible.config_dump(parsed_args) + expected_result = { + "host1": {"key1": "******", "key2": "value2", "key3": ["******"]}, + "host2": {"key1": {"key2": "******"}}, + } + self.assertEqual(result, expected_result) + dump_config_path = utils.get_data_files_path( + "ansible", "dump-config.yml") + mock_run.assert_called_once_with(parsed_args, + dump_config_path, + extra_vars={ + "dump_path": dump_dir, + }, + check_output=True, tags=None, + verbose_level=None, check=False, + list_tasks=False, diff=False) + mock_rmtree.assert_called_once_with(dump_dir) + mock_listdir.assert_any_call(dump_dir) + mock_read.assert_has_calls([ + mock.call(os.path.join(dump_dir, "host1.yml")), + mock.call(os.path.join(dump_dir, "host2.yml")), + ]) + @mock.patch.object(utils, 'galaxy_role_install', autospec=True) @mock.patch.object(utils, 'is_readable_file', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) diff --git a/kayobe/tests/unit/test_utils.py b/kayobe/tests/unit/test_utils.py index 89263ef82..cd126cd4b 100644 --- a/kayobe/tests/unit/test_utils.py +++ b/kayobe/tests/unit/test_utils.py @@ -17,6 +17,7 @@ import subprocess import unittest from unittest import mock +from ansible.parsing.yaml.objects import AnsibleVaultEncryptedUnicode import yaml from kayobe import exception @@ -127,6 +128,59 @@ key2: value2 mock_read.return_value = "[1{!" self.assertRaises(SystemExit, utils.read_yaml_file, "/path/to/file") + @mock.patch.object(utils, "read_file") + def test_read_config_dump_yaml_file(self, mock_read): + config = """--- +key1: value1 +key2: value2 +""" + mock_read.return_value = config + result = utils.read_config_dump_yaml_file("/path/to/file") + self.assertEqual(result, {"key1": "value1", "key2": "value2"}) + mock_read.assert_called_once_with("/path/to/file") + + @mock.patch.object(utils, "read_file") + def test_read_config_dump_yaml_file_vaulted(self, mock_read): + config = """--- +key1: !vault | + $ANSIBLE_VAULT;1.1;AES256 + 633230623736383232323862393364323037343430393530316636363961626361393133646437 + 643438663261356433656365646138666133383032376532310a63323432306431303437623637 + 346236316161343635636230613838316566383933313338636237616338326439616536316639 + 6334343462333062363334300a3930313762313463613537626531313230303731343365643766 + 666436333037 +key2: value2 +key3: + - !vault | + $ANSIBLE_VAULT;1.1;AES256 + 633230623736383232323862393364323037343430393530316636363961626361393133646437 + 643438663261356433656365646138666133383032376532310a63323432306431303437623637 + 346236316161343635636230613838316566383933313338636237616338326439616536316639 + 6334343462333062363334300a3930313762313463613537626531313230303731343365643766 + 666436333037 +""" + mock_read.return_value = config + result = utils.read_config_dump_yaml_file("/path/to/file") + # Can't read the value without an encryption key, so just check type. + self.assertTrue(isinstance(result["key1"], + AnsibleVaultEncryptedUnicode)) + self.assertEqual(result["key2"], "value2") + self.assertTrue(isinstance(result["key3"][0], + AnsibleVaultEncryptedUnicode)) + mock_read.assert_called_once_with("/path/to/file") + + @mock.patch.object(utils, "read_file") + def test_read_config_dump_yaml_file_open_failure(self, mock_read): + mock_read.side_effect = IOError + self.assertRaises(SystemExit, utils.read_config_dump_yaml_file, + "/path/to/file") + + @mock.patch.object(utils, "read_file") + def test_read_config_dump_yaml_file_not_yaml(self, mock_read): + mock_read.return_value = "[1{!" + self.assertRaises(SystemExit, utils.read_config_dump_yaml_file, + "/path/to/file") + @mock.patch.object(subprocess, "check_call") def test_run_command(self, mock_call): output = utils.run_command(["command", "to", "run"]) diff --git a/kayobe/utils.py b/kayobe/utils.py index ba9fdaad9..04094db48 100644 --- a/kayobe/utils.py +++ b/kayobe/utils.py @@ -24,6 +24,7 @@ import shutil import subprocess import sys +from ansible.parsing.yaml.loader import AnsibleLoader import yaml from kayobe import exception @@ -153,11 +154,28 @@ def read_yaml_file(path): try: content = read_file(path) except IOError as e: - print("Failed to open config dump file %s: %s" % + print("Failed to open YAML file %s: %s" % (path, repr(e))) sys.exit(1) try: return yaml.safe_load(content) + except yaml.YAMLError as e: + print("Failed to decode YAML file %s: %s" % + (path, repr(e))) + sys.exit(1) + + +def read_config_dump_yaml_file(path): + """Read and decode a configuration dump YAML file.""" + try: + content = read_file(path) + except IOError as e: + print("Failed to open config dump file %s: %s" % + (path, repr(e))) + sys.exit(1) + try: + # AnsibleLoader supports loading vault encrypted variables. + return AnsibleLoader(content).get_single_data() except yaml.YAMLError as e: print("Failed to decode config dump YAML file %s: %s" % (path, repr(e))) diff --git a/releasenotes/notes/config-dump-vault-edc615e475f234ac.yaml b/releasenotes/notes/config-dump-vault-edc615e475f234ac.yaml new file mode 100644 index 000000000..f8e6a2fc7 --- /dev/null +++ b/releasenotes/notes/config-dump-vault-edc615e475f234ac.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes an issue where ``kayobe configuration dump`` would fail when + variables are encrypted using Ansible Vault. Encrypted variables are now + sanitised in the dump output. `LP#2031390 + `__