From 569e49088fd1406b977421a6f9de314ab6e92402 Mon Sep 17 00:00:00 2001 From: Christoph Kuhnke Date: Thu, 28 Mar 2024 08:24:25 +0100 Subject: [PATCH] Feature/#255 change owner of notebooks to jupyter in entrypoint py (#265) * Updated changelog * Added unit tests * Implemented integration test * Apply suggestions from code review * Updated version to 2.0.0 * [run-notebook-tests] Co-authored-by: Torsten Kilias --- doc/changes/changelog.md | 2 +- doc/changes/changes_1.1.0.md | 39 --------------- doc/changes/changes_2.0.0.md | 49 +++++++++++++++++++ doc/developer_guide/testing.md | 14 +++--- doc/user_guide/docker/docker-usage.md | 2 +- doc/user_guide/vm-edition/win-vbox.md | 2 +- .../ds/sandbox/lib/ansible/ansible_runner.py | 2 +- .../roles/entrypoint/files/entrypoint.py | 14 ++++++ pyproject.toml | 2 +- test/docker/container.py | 14 +++++- test/integration/docker_socket_and_groups.py | 12 +++++ .../test_create_dss_docker_image.py | 48 +++++++++++++++++- .../test_notebooks_in_dss_docker_image.py | 1 + test/unit/entrypoint/test_main.py | 25 ++++++++++ test/unit/entrypoint/test_user_class.py | 17 +++++++ 15 files changed, 189 insertions(+), 54 deletions(-) delete mode 100644 doc/changes/changes_1.1.0.md create mode 100644 doc/changes/changes_2.0.0.md diff --git a/doc/changes/changelog.md b/doc/changes/changelog.md index 8b873cbe..87f2112b 100644 --- a/doc/changes/changelog.md +++ b/doc/changes/changelog.md @@ -1,6 +1,6 @@ # Changes -* [1.1.0](changes_1.1.0.md) +* [2.0.0](changes_2.0.0.md) * [1.0.0](changes_1.0.0.md) * [0.2.0](changes_0.2.0.md) * [0.1.0](changes_0.1.0.md) diff --git a/doc/changes/changes_1.1.0.md b/doc/changes/changes_1.1.0.md deleted file mode 100644 index 35d9e349..00000000 --- a/doc/changes/changes_1.1.0.md +++ /dev/null @@ -1,39 +0,0 @@ -# AI-Lab 1.1.0 released tbd - -Code name: TBD - -## Summary - -tbd - -## AI-Lab-Release - -Version: 1.1.0 - -## Features - -* #223: Added support to add docker image tag "latest" -* #204: Updated developer guide -* #177: Disabled core dumps - -## Security - -n/a - -## Bug Fixes - -* #241: Fixed non-root-user access - -## Documentation - -* #204: Updated developer guide -* #219: Described Virtual Box setup in user guide - -## Refactoring - -* #217: Changed notebook-connector dependency, now installing it from PyPi. -* #220: Changed default ports in the external database configuration. -* #221: Changed wording in the main configuration notebook, as suggested by PM. -* #66: Used a non-root user to run Jupyter in the Docker Image ai-lab -* #149: Split AWS tests -* #252: Added tests for access to Docker socket diff --git a/doc/changes/changes_2.0.0.md b/doc/changes/changes_2.0.0.md new file mode 100644 index 00000000..06d229cb --- /dev/null +++ b/doc/changes/changes_2.0.0.md @@ -0,0 +1,49 @@ +# AI-Lab 2.0.0 released 2024-03-28 + +Code name: Use non-privileged user for running JupyterLab + +## Summary + +The following changes are especially important if you are using the AI-Lab's Docker Edition and are [mounting a volume](../user_guide/docker/managing-user-data.md) containing your private notebook files and the [Secure Configuration Storage](../user_guide/docker/secure-configuration-storage.md) (SCS) into the AI-Lab's Docker container. + +Major changes + +1. The mount-point for Jupyter notebook files and the SCS has moved from `/root/notebooks` to `/home/jupyter/notebooks`. +2. Some of the notebooks have been updated, especially the Cloud Storage Extension notebook. + +In case you are using the AI-Lab's Docker Edition with mounted volume, then please +1. Change your commands to use the new mount point as described in the [User Guide](../user_guide/docker/docker-usage.md#creating-a-docker-container-for-the-ai--lab-from-the-ai-lab-docker-image) and +2. Find the updated notebooks in folder `/home/jupyter/notebook-defaults` as the AI-Lab does not overwrite existing files, to avoid losing manual changes. + +## AI-Lab-Release + +Version: 2.0.0 + +## Features + +* #223: Added support to add docker image tag "latest" +* #204: Updated developer guide +* #177: Disabled core dumps +* #255: Changed owner of notebooks to jupyter in `entrypoint.py` + +## Security + +n/a + +## Bug Fixes + +* #241: Fixed non-root-user access + +## Documentation + +* #204: Updated developer guide +* #219: Described Virtual Box setup in user guide + +## Refactoring + +* #217: Changed notebook-connector dependency, now installing it from PyPi. +* #220: Changed default ports in the external database configuration. +* #221: Changed wording in the main configuration notebook, as suggested by PM. +* #66: Used a non-root user to run Jupyter in the Docker Image ai-lab +* #149: Split AWS tests +* #252: Added tests for access to Docker socket diff --git a/doc/developer_guide/testing.md b/doc/developer_guide/testing.md index 95b5082a..33d15ad4 100644 --- a/doc/developer_guide/testing.md +++ b/doc/developer_guide/testing.md @@ -1,17 +1,17 @@ ### Tests -XAL comes with a number of tests in directory `test`. -Besides, unit and integrations tests in the respective directories -there are tests in directory `codebuild`, see [Executing AWS CodeBuild](ci.md#executing-aws-codebuild). +XAL comes with a number of tests in directory `test`. +Besides, unit and integrations tests in the respective directories +there are tests in directory `codebuild`, see [Executing AWS CodeBuild](ci.md#executing-aws-codebuild). # Speeding up Docker-based Tests -Creating a docker image is quite time-consuming, currently around 7 minutes. In order to use an existing -docker image in the tests in `integration/test_create_dss_docker_image.py` +Creating a docker image is quite time-consuming, currently around 7 minutes. In order to use an existing +docker image in the tests in `integration/test_create_dss_docker_image.py` simply add CLI option `--dss-docker-image` when calling `pytest`: -```shell -poetry run pytest --dss-docker-image exasol/ai-lab:1.0.0 +```shell +poetry run pytest --dss-docker-image exasol/ai-lab:2.0.0 ``` #### Executing tests involving AWS resources diff --git a/doc/user_guide/docker/docker-usage.md b/doc/user_guide/docker/docker-usage.md index 5682b556..4679763f 100644 --- a/doc/user_guide/docker/docker-usage.md +++ b/doc/user_guide/docker/docker-usage.md @@ -20,7 +20,7 @@ The Unix shell commands in the following sections will use some environment vari Here is an example: ```shell -VERSION=1.0.0 +VERSION=2.0.0 LISTEN_IP=0.0.0.0 VOLUME=my-vol CONTAINER_NAME=ai-lab diff --git a/doc/user_guide/vm-edition/win-vbox.md b/doc/user_guide/vm-edition/win-vbox.md index 07b4771a..1fe35d76 100644 --- a/doc/user_guide/vm-edition/win-vbox.md +++ b/doc/user_guide/vm-edition/win-vbox.md @@ -9,7 +9,7 @@ ## Select Virtual machine Name and Operating System * Create a new virtual machine -* Enter a name for your virtual machine, e.g. "Exasol-AI-Lab-1.0.0" +* Enter a name for your virtual machine, e.g. "Exasol-AI-Lab-2.0.0" * Select a folder to store the VM image to * Select operating system "Linux", e.g. version "Ubuntu 22.04" * Click button "Next" diff --git a/exasol/ds/sandbox/lib/ansible/ansible_runner.py b/exasol/ds/sandbox/lib/ansible/ansible_runner.py index 0f16bac1..d1147337 100644 --- a/exasol/ds/sandbox/lib/ansible/ansible_runner.py +++ b/exasol/ds/sandbox/lib/ansible/ansible_runner.py @@ -48,7 +48,7 @@ def event_handler(self, event: AnsibleEvent) -> bool: if not "event_data" in event: return True duration = event["event_data"].get("duration", 0) - if duration > 0.5: + if duration > 1.5: self._duration_logger.debug(f"duration: {round(duration)} seconds") return True diff --git a/exasol/ds/sandbox/runtime/ansible/roles/entrypoint/files/entrypoint.py b/exasol/ds/sandbox/runtime/ansible/roles/entrypoint/files/entrypoint.py index 1005a13e..dffdf4b0 100644 --- a/exasol/ds/sandbox/runtime/ansible/roles/entrypoint/files/entrypoint.py +++ b/exasol/ds/sandbox/runtime/ansible/roles/entrypoint/files/entrypoint.py @@ -280,6 +280,18 @@ def id(self): self._id = pwd.getpwnam(self.name).pw_uid return self._id + def chown_recursive(self, path: Path): + uid = self.id + gid = self.group.id + os.chown(path, uid, gid) + for root, dirs, files in os.walk(path): + root = Path(root) + for name in files: + os.chown(root / name, uid, gid) + for name in dirs: + os.chown(root / name, uid, gid) + _logger.info(f"Did chown -R {self.name}:{self.group.name} {path}") + def enable_group_access(self, path: Path): file = FileInspector(path) if file.is_group_accessible(): @@ -310,6 +322,8 @@ def main(): args = arg_parser().parse_args() user = User(args.user, Group(args.group), Group(args.docker_group)) if user.is_specified: + if args.notebooks: + user.chown_recursive(args.notebooks) user.enable_group_access(Path("/var/run/docker.sock")).switch_to() if args.notebook_defaults and args.notebooks: copy_rec( diff --git a/pyproject.toml b/pyproject.toml index 138a3b5c..a2d20126 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "exasol-ai-lab" -version = "1.1.0" +version = "2.0.0" description = "Provide AI-Lab editions." packages = [ {include = "exasol"}, ] license = "MIT" diff --git a/test/docker/container.py b/test/docker/container.py index 63c92b90..c1f69313 100644 --- a/test/docker/container.py +++ b/test/docker/container.py @@ -1,3 +1,4 @@ +import logging import re import docker @@ -13,6 +14,9 @@ from docker.models.images import Image +_logger = logging.getLogger(__name__) + + def sanitize_container_name(test_name: str): test_name = re.sub('[^0-9a-zA-Z-]+', '_', test_name) test_name = re.sub('_+', '_', test_name) @@ -82,17 +86,23 @@ def wait_for( DOCKER_SOCKET_CONTAINER = "/var/run/docker.sock" -def wait_for_socket_access(container: Container): +def wait_for_socket_access( + container: Container, + timeout: timedelta = timedelta(seconds=5), +): wait_for( container, f"entrypoint.py: Enabled access to {DOCKER_SOCKET_CONTAINER}", + timeout, ) def assert_exec_run(container: Container, command: str, **kwargs) -> str: """ - Execute command in container and verify success. + Execute command in container, verify its success, and return + utf-8-decoded ouput. """ + _logger.debug(f'Running command in Docker container: {command}') exit_code, output = container.exec_run(command, **kwargs) output = output.decode("utf-8").strip() assert exit_code == 0, output diff --git a/test/integration/docker_socket_and_groups.py b/test/integration/docker_socket_and_groups.py index 3aef0be6..2db65386 100644 --- a/test/integration/docker_socket_and_groups.py +++ b/test/integration/docker_socket_and_groups.py @@ -70,6 +70,18 @@ def chgrp(self, gid: int, path_on_host: Path): with self._context_provider(path_on_host, path_in_container) as container: assert_exec_run(container, f"chgrp {gid} {path_in_container}") + def chown_chmod_recursive( + self, + owner: str, + permissions: str, + path_on_host: Path, + ): + """`owner` may be specifed as user or user:group""" + path_in_container = "/mounted" + with self._context_provider(path_on_host, path_in_container) as container: + assert_exec_run(container, f"chown -R {owner} {path_in_container}") + assert_exec_run(container, f"chmod -R {permissions} {path_in_container}") + @contextmanager def dss_image_with_added_group(request, base_image, gid, group_name): diff --git a/test/integration/test_create_dss_docker_image.py b/test/integration/test_create_dss_docker_image.py index f26d7e63..972c8f97 100644 --- a/test/integration/test_create_dss_docker_image.py +++ b/test/integration/test_create_dss_docker_image.py @@ -18,7 +18,7 @@ from re import Pattern from tenacity.wait import wait_fixed from tenacity.stop import stop_after_delay -from typing import Set, Tuple +from typing import List, Set, Tuple from datetime import datetime, timedelta from exasol.ds.sandbox.lib.logging import set_log_level @@ -255,3 +255,49 @@ def test_write_socket_known_gid( with SocketInspector(request, image.id, socket_on_host) as inspector: inspector.assert_jupyter_member_of(group_name) inspector.assert_write_to_socket() + + +def docker_image_getenv(image_name: str, variable: str) -> str: + client = docker.from_env() + image = client.images.get(image_name) + client.close() + def pair(entry: str) -> Tuple[str,str]: + parts = entry.partition("=") + return parts[0], parts[2], + + env = dict([pair(e) for e in image.attrs["Config"]["Env"]]) + return env.get(variable, None) + + +def test_chown_notebooks(request, tmp_path, group_changer, dss_docker_image): + def ls_command(old_path: str, new_path: str, args: List[Path]) -> str: + args = (str(p).replace(old_path, new_path) for p in args) + return "ls -ld " + " ".join(args) + + def user_and_group(ls_line: str) -> str: + columns = ls_line.split() + return f"{columns[2]}:{columns[3]}" + + child = tmp_path / "child" + sub = tmp_path / "sub" + grand_child = sub / "grand_child" + child.touch() + sub.mkdir() + grand_child.touch() + group_changer.chown_chmod_recursive("root:root", "777", tmp_path) + + notebooks_folder = docker_image_getenv( + dss_docker_image.image_name, + "NOTEBOOK_FOLDER_FINAL") + with container_context( + request, + image_name=dss_docker_image.image_name, + volumes={ tmp_path: { + 'bind': notebooks_folder, + 'mode': 'rw', }, }, + ) as container: + testees = [tmp_path, child, sub, grand_child] + command = ls_command(str(tmp_path), notebooks_folder, testees) + output = assert_exec_run(container, command) + for line in output.splitlines(): + assert "jupyter:jupyter" == user_and_group(line) diff --git a/test/notebook_test_runner/test_notebooks_in_dss_docker_image.py b/test/notebook_test_runner/test_notebooks_in_dss_docker_image.py index 4a9f1f0c..dec0aced 100644 --- a/test/notebook_test_runner/test_notebooks_in_dss_docker_image.py +++ b/test/notebook_test_runner/test_notebooks_in_dss_docker_image.py @@ -4,6 +4,7 @@ import time from inspect import cleandoc from pathlib import Path +from datetime import timedelta import pytest diff --git a/test/unit/entrypoint/test_main.py b/test/unit/entrypoint/test_main.py index 6b2ecfac..002c4908 100644 --- a/test/unit/entrypoint/test_main.py +++ b/test/unit/entrypoint/test_main.py @@ -10,8 +10,16 @@ def test_no_args(mocker): mocker.patch("sys.argv", ["app"]) mocker.patch(entrypoint_method("sleep_infinity")) + mocker.patch(entrypoint_method("start_jupyter_server")) + mocker.patch(entrypoint_method("copy_rec")) + user = create_autospec(entrypoint.User, is_specified=False) + mocker.patch(entrypoint_method("User"), return_value=user) entrypoint.main() assert entrypoint.sleep_infinity.called + assert not user.enable_group_access.called + assert not user.chown_recursive.called + assert not entrypoint.copy_rec.called + assert not entrypoint.start_jupyter_server.called def test_user_arg(mocker): @@ -37,6 +45,23 @@ def test_user_arg(mocker): assert user.switch_to.called +def test_chown_recursive_args(mocker): + dir = "/path/to/final/notebooks" + mocker.patch("sys.argv", [ + "app", + "--user", "jennifer", + "--group", "users", + "--docker-group", "docker", + "--notebooks", dir, + ]) + user = create_autospec(entrypoint.User) + mocker.patch(entrypoint_method("User"), return_value=user) + mocker.patch(entrypoint_method("sleep_infinity")) + entrypoint.main() + assert user.chown_recursive.called + assert user.chown_recursive.call_args == mocker.call(Path(dir)) + + @pytest.mark.parametrize("warning_as_error", [True, False]) def test_copy_args_valid(mocker, warning_as_error ): extra_args = ["--warning-as-error"] if warning_as_error else [] diff --git a/test/unit/entrypoint/test_user_class.py b/test/unit/entrypoint/test_user_class.py index 92a1c64b..9799575b 100644 --- a/test/unit/entrypoint/test_user_class.py +++ b/test/unit/entrypoint/test_user_class.py @@ -56,6 +56,23 @@ def test_uid(mocker, user): and pwd.getpwnam.call_args == mocker.call("jennifer") +def test_chown_recursive(mocker, user, tmp_path): + child = tmp_path / "child" + sub = tmp_path / "sub" + grand_child = sub / "grand_child" + child.touch() + sub.mkdir() + grand_child.touch() + mocker.patch("os.chown") + passwd_struct = MagicMock(pw_uid=444) + mocker.patch("pwd.getpwnam", return_value=passwd_struct) + user.chown_recursive(tmp_path) + expected = [ mocker.call(f, user.id, user.group.id) for f in ( + tmp_path, child, sub, grand_child, + )] + assert expected == os.chown.call_args_list + + def test_enable_file_absent(mocker, user): mocker.patch(entrypoint_method("GroupAccess")) user.enable_group_access(Path("/non/existing/path"))