From 8930a3c1066e47c743e8f8bf2e6bd1ba03487aa7 Mon Sep 17 00:00:00 2001
From: Mark Goddard <mark@stackhpc.com>
Date: Fri, 8 Dec 2023 09:35:52 +0000
Subject: [PATCH] Prevent running from a different Kayobe configuration
repository
There are various ways in which it is possible to operate Kayobe
incorrectly. One example is executing Kayobe from a different Kayobe
configuration repository than the one referred to by the
KAYOBE_CONFIG_PATH environment variable.
While this shouldn't necessarily cause any errors, it may lead to
unexpected results if the operator assumes they are operating against
the configuration in the current directory.
This change adds a validation step that checks for this case and fails
the command early if found.
Change-Id: I709884bbd7edebf1d409f39c11f293560e987506
---
kayobe/cli/commands.py | 8 ++
kayobe/tests/unit/test_utils.py | 128 +++++++++++++++++-
kayobe/utils.py | 62 ++++++++-
...validate-config-path-f26550903c1eb82a.yaml | 7 +
4 files changed, 195 insertions(+), 10 deletions(-)
create mode 100644 releasenotes/notes/validate-config-path-f26550903c1eb82a.yaml
diff --git a/kayobe/cli/commands.py b/kayobe/cli/commands.py
index 170a5698..5231a0b0 100644
--- a/kayobe/cli/commands.py
+++ b/kayobe/cli/commands.py
@@ -237,7 +237,15 @@ class HookDispatcher(CommandHook):
self.logger.debug("Running hooks: %s" % hooks)
self.command.run_kayobe_playbooks(parsed_args, hooks)
+ def _preflight_checks(self, parsed_args):
+ # NOTE(mgoddard): Currently all commands use KayobeAnsibleMixin, so
+ # should have a config_path attribute, but better to be defensive.
+ config_path = getattr(parsed_args, "config_path", None)
+ if config_path:
+ utils.validate_config_path(config_path)
+
def before(self, parsed_args):
+ self._preflight_checks(parsed_args)
self.run_hooks(parsed_args, "pre")
return parsed_args
diff --git a/kayobe/tests/unit/test_utils.py b/kayobe/tests/unit/test_utils.py
index 88fb96da..0d31f39d 100644
--- a/kayobe/tests/unit/test_utils.py
+++ b/kayobe/tests/unit/test_utils.py
@@ -12,6 +12,7 @@
# License for the specific language governing permissions and limitations
# under the License.
+import logging
import os
import subprocess
import unittest
@@ -189,14 +190,12 @@ key3:
mock_call.assert_called_once_with(["command", "to", "run"])
self.assertIsNone(output)
- @mock.patch("kayobe.utils.open")
@mock.patch.object(subprocess, "check_call")
- def test_run_command_quiet(self, mock_call, mock_open):
- mock_devnull = mock_open.return_value.__enter__.return_value
+ def test_run_command_quiet(self, mock_call):
output = utils.run_command(["command", "to", "run"], quiet=True)
mock_call.assert_called_once_with(["command", "to", "run"],
- stdout=mock_devnull,
- stderr=mock_devnull)
+ stdout=subprocess.DEVNULL,
+ stderr=subprocess.DEVNULL)
self.assertIsNone(output)
@mock.patch.object(subprocess, "check_output")
@@ -332,3 +331,122 @@ key3:
finder = utils.EnvironmentFinder('/etc/kayobe', None)
self.assertEqual([], finder.ordered())
self.assertEqual([], finder.ordered_paths())
+
+ @mock.patch.object(utils, "run_command")
+ @mock.patch.object(utils, "is_readable_file")
+ def test_validate_config_path_kayobe(self, mock_readable, mock_run):
+ mock_run.return_value = b"/path/to/config"
+ utils.validate_config_path("/path/to/config/etc/kayobe")
+ mock_run.assert_called_once_with(
+ ["git", "rev-parse", "--show-toplevel"],
+ check_output=True, quiet=True)
+ self.assertFalse(mock_readable.called)
+
+ @mock.patch.object(utils, "run_command")
+ @mock.patch.object(utils, "is_readable_file")
+ def test_validate_config_path_not_a_repo(self, mock_readable, mock_run):
+ mock_run.side_effect = subprocess.CalledProcessError(
+ "not a repo", "command")
+ utils.validate_config_path("/path/to/config/etc/kayobe")
+ mock_run.assert_called_once_with(
+ ["git", "rev-parse", "--show-toplevel"],
+ check_output=True, quiet=True)
+ self.assertFalse(mock_readable.called)
+
+ @mock.patch.object(utils, "run_command")
+ @mock.patch.object(utils, "is_readable_file")
+ def test_validate_config_path_no_git(self, mock_readable, mock_run):
+ mock_run.side_effect = FileNotFoundError("No such file")
+ utils.validate_config_path("/path/to/config/etc/kayobe")
+ mock_run.assert_called_once_with(
+ ["git", "rev-parse", "--show-toplevel"],
+ check_output=True, quiet=True)
+ self.assertFalse(mock_readable.called)
+
+ @mock.patch.object(utils, "run_command")
+ @mock.patch.object(utils, "is_readable_file")
+ @mock.patch.object(utils, "read_file")
+ def test_validate_config_path_gitreview(self, mock_read, mock_readable,
+ mock_run):
+ mock_run.return_value = b"/path/to/repo"
+ mock_readable.return_value = {"result": True}
+ mock_read.return_value = """
+[gerrit]
+project=openstack/kayobe-config.git
+"""
+ with self.assertLogs(level=logging.ERROR) as ctx:
+ self.assertRaises(SystemExit,
+ utils.validate_config_path,
+ "/path/to/config/etc/kayobe")
+ exp = ("Executing from within a different Kayobe configuration "
+ "repository is not allowed")
+ assert any(exp in t for t in ctx.output)
+ mock_run.assert_called_once_with(
+ ["git", "rev-parse", "--show-toplevel"],
+ check_output=True, quiet=True)
+ mock_readable.assert_called_once_with("/path/to/repo/.gitreview")
+ mock_read.assert_called_once_with("/path/to/repo/.gitreview")
+
+ @mock.patch.object(utils, "run_command")
+ @mock.patch.object(utils, "is_readable_file")
+ def test_validate_config_path_no_gitreview(self, mock_readable, mock_run):
+ mock_run.return_value = b"/path/to/repo"
+ mock_readable.return_value = {"result": False}
+ utils.validate_config_path("/path/to/config/etc/kayobe")
+ mock_run.assert_called_once_with(
+ ["git", "rev-parse", "--show-toplevel"],
+ check_output=True, quiet=True)
+ mock_readable.assert_called_once_with("/path/to/repo/.gitreview")
+
+ @mock.patch.object(utils, "run_command")
+ @mock.patch.object(utils, "is_readable_file")
+ @mock.patch.object(utils, "read_file")
+ def test_validate_config_path_gitreview_no_gerrit(self, mock_read,
+ mock_readable, mock_run):
+ mock_run.return_value = b"/path/to/repo"
+ mock_readable.return_value = {"result": False}
+ mock_read.return_value = """
+[foo]
+bar=baz
+"""
+ utils.validate_config_path("/path/to/config/etc/kayobe")
+ mock_run.assert_called_once_with(
+ ["git", "rev-parse", "--show-toplevel"],
+ check_output=True, quiet=True)
+ mock_readable.assert_called_once_with("/path/to/repo/.gitreview")
+
+ @mock.patch.object(utils, "run_command")
+ @mock.patch.object(utils, "is_readable_file")
+ @mock.patch.object(utils, "read_file")
+ def test_validate_config_path_gitreview_no_project(self, mock_read,
+ mock_readable,
+ mock_run):
+ mock_run.return_value = b"/path/to/repo"
+ mock_readable.return_value = {"result": False}
+ mock_read.return_value = """
+[gerrit]
+bar=baz
+"""
+ utils.validate_config_path("/path/to/config/etc/kayobe")
+ mock_run.assert_called_once_with(
+ ["git", "rev-parse", "--show-toplevel"],
+ check_output=True, quiet=True)
+ mock_readable.assert_called_once_with("/path/to/repo/.gitreview")
+
+ @mock.patch.object(utils, "run_command")
+ @mock.patch.object(utils, "is_readable_file")
+ @mock.patch.object(utils, "read_file")
+ def test_validate_config_path_gitreview_other_project(self, mock_read,
+ mock_readable,
+ mock_run):
+ mock_run.return_value = b"/path/to/repo"
+ mock_readable.return_value = {"result": False}
+ mock_read.return_value = """
+[gerrit]
+project=baz
+"""
+ utils.validate_config_path("/path/to/config/etc/kayobe")
+ mock_run.assert_called_once_with(
+ ["git", "rev-parse", "--show-toplevel"],
+ check_output=True, quiet=True)
+ mock_readable.assert_called_once_with("/path/to/repo/.gitreview")
diff --git a/kayobe/utils.py b/kayobe/utils.py
index 4d10e924..f53d8e2b 100644
--- a/kayobe/utils.py
+++ b/kayobe/utils.py
@@ -14,6 +14,7 @@
import base64
from collections import defaultdict
+import configparser
import glob
import graphlib
from importlib.metadata import Distribution
@@ -223,11 +224,10 @@ def run_command(cmd, quiet=False, check_output=False, **kwargs):
cmd_string = " ".join(cmd)
LOG.debug("Running command: %s", cmd_string)
if quiet:
- with open("/dev/null", "w") as devnull:
- kwargs["stdout"] = devnull
- kwargs["stderr"] = devnull
- subprocess.check_call(cmd, **kwargs)
- elif check_output:
+ kwargs["stderr"] = subprocess.DEVNULL
+ if not check_output:
+ kwargs["stdout"] = subprocess.DEVNULL
+ if check_output:
return subprocess.check_output(cmd, **kwargs)
else:
subprocess.check_call(cmd, **kwargs)
@@ -409,3 +409,55 @@ class EnvironmentFinder(object):
)
result.append(full_path)
return result
+
+
+def _gitreview_is_kayobe_config(gitreview_path):
+ """Return whether a .gitreview file is for kayobe-config."""
+ config = configparser.ConfigParser()
+ config_string = read_file(gitreview_path)
+ config.read_string(config_string)
+ gerrit_project = config.get('gerrit', 'project')
+ if not gerrit_project:
+ return False
+ gerrit_project = os.path.basename(gerrit_project)
+ gerrit_project = os.path.splitext(gerrit_project)[0]
+ if gerrit_project == 'kayobe-config':
+ return True
+
+
+def validate_config_path(config_path):
+ """Validate the Kayobe configuration path.
+
+ Check whether we are executing from inside a Kayobe configuration
+ repository, and if so, assert that matches the Kayobe configuration path
+ defined in CLI args or environment variables.
+
+ Exit 1 if validation fails.
+
+ :param config_path: Kayobe configuration path or None.
+ """
+ assert config_path
+
+ try:
+ cmd = ["git", "rev-parse", "--show-toplevel"]
+ repo_root = run_command(cmd, quiet=True, check_output=True)
+ except (FileNotFoundError, subprocess.CalledProcessError):
+ # FileNotFoundError: git probably not installed.
+ # CalledProcessError: probably not in a git repository.
+ return
+
+ repo_root = repo_root.decode().strip()
+ if config_path:
+ repo_config_path = os.path.join(repo_root, "etc", "kayobe")
+ if repo_config_path == os.path.realpath(config_path):
+ return
+
+ # Paths did not match. Check that repo_root does not look like a Kayobe
+ # configuration repo.
+ gitreview_path = os.path.join(repo_root, ".gitreview")
+ result = is_readable_file(gitreview_path)
+ if result["result"]:
+ if _gitreview_is_kayobe_config(gitreview_path):
+ LOG.error("Executing from within a different Kayobe configuration "
+ "repository is not allowed")
+ sys.exit(1)
diff --git a/releasenotes/notes/validate-config-path-f26550903c1eb82a.yaml b/releasenotes/notes/validate-config-path-f26550903c1eb82a.yaml
new file mode 100644
index 00000000..ed99e10b
--- /dev/null
+++ b/releasenotes/notes/validate-config-path-f26550903c1eb82a.yaml
@@ -0,0 +1,7 @@
+---
+features:
+ - |
+ Adds validation to protect against executing Kayobe from within a different
+ Kayobe configuration repository than the one referred to by environment
+ variables (e.g. ``KAYOBE_CONFIG_PATH``) or CLI arguments (e.g.
+ ``--config-path``).
--
GitLab