From 78702d0e3094e6d6a16a31eaf2517d4e0f25d1c7 Mon Sep 17 00:00:00 2001
From: Mark Goddard <mark@stackhpc.com>
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 <host>'
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'", <yaml.error.Mark object at 0x7f1e5c7404c0>)

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 fed57ea4..9dc7cb4d 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 87568841..40b2437d 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 89263ef8..cd126cd4 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 ba9fdaad..04094db4 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 00000000..f8e6a2fc
--- /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
+    <https://bugs.launchpad.net/kayobe/+bug/2031390>`__
-- 
GitLab