Skip to content

Commit

Permalink
testlib.containers: Run tests as testuser
Browse files Browse the repository at this point in the history
The dockerized tests have been run as user root so far, which
caused some trouble like raising several MKGeneralException errors
when running the unit tests and is generally not recommended for
any integration and system tests.

This change fixes that by running the tests as testuser and making
the necessary adaptions to make this work.

Change-Id: I28be44ec305693e7c2e2e3e9a521d17f5c29348d
  • Loading branch information
rene-slowenski-checkmk committed Aug 23, 2024
1 parent 84f23bf commit b892d3c
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 23 deletions.
5 changes: 4 additions & 1 deletion tests/composition/agents/test_cmk_agent_ctl.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@
# This file is part of Checkmk (https://checkmk.com). It is subject to the terms and
# conditions defined in the file COPYING, which is part of this source code package.

import logging
from pathlib import Path

from tests.testlib.pytest_helpers.marks import skip_if_not_containerized
from tests.testlib.utils import run

logger = logging.getLogger(__name__)


@skip_if_not_containerized
def test_agent_controller_installed(agent_ctl: Path) -> None:
Expand All @@ -17,5 +20,5 @@ def test_agent_controller_installed(agent_ctl: Path) -> None:

@skip_if_not_containerized
def test_dump(agent_ctl: Path) -> None:
res = run(["sudo", agent_ctl.as_posix(), "dump"])
res = run([agent_ctl.as_posix(), "dump"], sudo=True)
assert res.stdout.startswith("<<<check_mk>>>")
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,13 @@ def test_proxy_register_import_workflow(
central_site.openapi.create_host(hostname=hostname, attributes={"ipaddress": "127.0.0.1"})
central_site.openapi.activate_changes_and_wait_for_completion()

run(
[agent_ctl.as_posix(), "delete-all"],
check=False,
sudo=True,
)
proxy_registration_proc = run(
[
"sudo",
agent_ctl.as_posix(),
"proxy-register",
"--server",
Expand All @@ -42,7 +46,8 @@ def test_proxy_register_import_workflow(
"--password",
central_site.admin_password,
"--trust-cert",
]
],
sudo=True,
)
subprocess.run(
["sudo", agent_ctl.as_posix(), "import"],
Expand Down
23 changes: 12 additions & 11 deletions tests/scripts/agent_controller_daemon.py
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -54,20 +54,21 @@ def _provide_agent_unix_socket() -> Iterator[None]:
socket_address.unlink(missing_ok=True)


def _run_controller_daemon(ctl_path: Path) -> int:
def _run_controller_daemon(ctl_path: Path) -> Iterator[subprocess.Popen]:
with subprocess.Popen(
[ctl_path.as_posix(), "daemon"],
stderr=subprocess.PIPE,
stdout=subprocess.PIPE,
close_fds=True,
encoding="utf-8",
) as proc:
exit_code = proc.wait()
stdout, stderr = proc.communicate()
logger.info("Stdout from controller daemon process:\n%s", stdout)
logger.info("Stderr from controller daemon process:\n%s", stderr)

return exit_code
try:
yield proc
finally:
stdout, stderr = proc.communicate()
proc.kill()
logger.info("Stdout from controller daemon process:\n%s", stdout)
logger.info("Stderr from controller daemon process:\n%s", stderr)


@contextlib.contextmanager
Expand All @@ -80,12 +81,12 @@ def _clean_agent_controller(ctl_path: Path) -> Iterator[None]:


@contextlib.contextmanager
def agent_controller_daemon(ctl_path: Path) -> Iterator[int]:
def agent_controller_daemon(ctl_path: Path) -> Iterator[subprocess.Popen]:
"""Manually take over systemds job if we are in a container (where we have no systemd)."""
with _clean_agent_controller(ctl_path), _provide_agent_unix_socket():
yield _run_controller_daemon(ctl_path)
yield from _run_controller_daemon(ctl_path)


if __name__ == "__main__":
with agent_controller_daemon(agent_controller_path) as rc:
sys.exit(rc)
with agent_controller_daemon(agent_controller_path) as daemon:
sys.exit(daemon.wait())
16 changes: 16 additions & 0 deletions tests/testlib/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,22 @@ def agent_controller_daemon(ctl_path: Path) -> Iterator[subprocess.Popen | None]

with execute(["python3", daemon_path, ctl_path.as_posix()], sudo=True) as daemon:
yield daemon
logger.info("Running agent controller daemon...")
with execute([daemon_path], sudo=True) as daemon:
# wait for a dump being returned successfully (may not work immediately after starting the agent controller)
wait_until(
lambda: execute([ctl_path.as_posix(), "dump"], sudo=True).wait() == 0,
timeout=3,
interval=0.1,
)
try:
yield daemon
finally:
assert daemon.returncode is None, "Daemon was killed unexpectedly!"
logger.info("Killing agent controller daemon...")
daemon.kill()
logger.info("daemon.stdout: %s", daemon.stdout)
logger.info("daemon.stderr: %s", daemon.stderr)


def register_controller(
Expand Down
39 changes: 31 additions & 8 deletions tests/testlib/containers.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
_DOCKER_REGISTRY_URL = "https://%s/v2/" % _DOCKER_REGISTRY
# Increase this to enforce invalidation of all existing images
_DOCKER_BUILD_ID = 1
_TESTUSER = "testuser"

logger = logging.getLogger()

Expand Down Expand Up @@ -79,8 +80,8 @@ def execute_tests_in_container(
stdin_open=True,
tty=True,
) as container:
# Ensure we can make changes to the git directory (not persisting it outside of the container)
_prepare_git_overlay(container, "/git-lowerdir", "/git")
_prepare_testuser(container, _TESTUSER)
_prepare_git_overlay(container, "/git-lowerdir", "/git", _TESTUSER)
_cleanup_previous_virtual_environment(container, version)
_reuse_persisted_virtual_environment(container, version)

Expand Down Expand Up @@ -127,6 +128,7 @@ def execute_tests_in_container(
container,
command,
check=False,
user=_TESTUSER,
environment=_container_env(version),
workdir="/git",
stream=True,
Expand Down Expand Up @@ -328,11 +330,6 @@ def _create_cmk_image(
logger.info(
"Building in container %s (from [%s])", container.short_id, base_image_name_with_tag
)
_exec_run(container, f"groupadd -g {os.getgid()} testuser")
_exec_run(container, f"useradd -m -u {os.getuid()} -g {os.getgid()} -s /bin/bash testuser")
_exec_run(container, "mkdir -p /home/testuser/.cache")
_exec_run(container, "mkdir -p /results")

# Ensure we can make changes to the git directory (not persisting it outside of the container)
_prepare_git_overlay(container, "/git-lowerdir", "/git")
_prepare_virtual_environment(container, version)
Expand Down Expand Up @@ -658,7 +655,9 @@ def _copy_directory(
tar.extractall(str(dest_path), filter="data")


def _prepare_git_overlay(container: docker.Container, lower_path: str, target_path: str) -> None:
def _prepare_git_overlay(
container: docker.Container, lower_path: str, target_path: str, username: str | None = None
) -> None:
"""Prevent modification of git checkout volume contents
Create some tmpfs that is mounted as rw layer over the the git checkout
Expand Down Expand Up @@ -707,6 +706,30 @@ def _prepare_git_overlay(container: docker.Container, lower_path: str, target_pa
],
)

if username:
_exec_run(container, ["chown", "-R", f"{username}:{username}", "/git"])


def _prepare_testuser(container: docker.Container, username: str) -> None:
"""Setup the environment for use with the testuser
Make sure all relevant files are owned by testuser and allow passwordless sudo, which is
required for tests that need to install packages or modify the system configuration.
"""
uid = str(os.getuid())
gid = str(os.getgid())
_exec_run(container, ["groupadd", "-g", gid, username], check=False)
_exec_run(
container, ["useradd", "-m", "-u", uid, "-g", gid, "-s", "/bin/bash", username], check=False
)
_exec_run(container, ["bash", "-c", f'echo "{username} ALL=(ALL) NOPASSWD: ALL">>/etc/sudoers'])

_exec_run(container, ["mkdir", "-p", f"/home/{username}/.cache"])
_exec_run(container, ["chown", "-R", f"{username}:{username}", f"/home/{username}"])

_exec_run(container, ["mkdir", "-p", "/results"])
_exec_run(container, ["chown", "-R", f"{username}:{username}", "/results"])


def _prepare_virtual_environment(container: docker.Container, version: CMKVersion) -> None:
"""Ensure the virtual environment is ready for use
Expand Down
2 changes: 1 addition & 1 deletion tests/testlib/site.py
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,7 @@ def save_results(self) -> None:
execute(["cp", nagios_log_path, (self.result_dir() / "log").as_posix()], sudo=True)

cmc_dir = self.result_dir() / "cmc"
os.makedirs(cmc_dir, exist_ok=True)
makedirs(cmc_dir, sudo=True)

execute(
["cp", self.path("var/check_mk/core/history"), (cmc_dir / "history").as_posix()],
Expand Down

0 comments on commit b892d3c

Please sign in to comment.