From cebd41b26425ea51a4087600bf73d0088df0bdd8 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Mon, 25 Oct 2021 16:25:33 +0300 Subject: [PATCH 1/3] Build: change docker container to set MONKEY_DOCKER_CONTAINER env var. This variable is needed because we can't prompt for data dir removal on docker like we do on other deployments Due to the fact that docker is not running interactively and user might be running on an old data dir if he uses volumes, we need special case for docker --- build_scripts/docker/Dockerfile | 1 + monkey/monkey_island/cc/setup/data_dir.py | 12 ++++++ monkey/monkey_island/cc/setup/env_utils.py | 8 ++++ .../monkey_island/cc/setup/test_data_dir.py | 38 ++++++++++++++++--- 4 files changed, 53 insertions(+), 6 deletions(-) create mode 100644 monkey/monkey_island/cc/setup/env_utils.py diff --git a/build_scripts/docker/Dockerfile b/build_scripts/docker/Dockerfile index 2637d3725..ecd2ce296 100644 --- a/build_scripts/docker/Dockerfile +++ b/build_scripts/docker/Dockerfile @@ -18,6 +18,7 @@ COPY --from=builder /monkey /monkey WORKDIR /monkey EXPOSE 5000 EXPOSE 5001 +ENV MONKEY_DOCKER_CONTAINER=true RUN groupadd -r monkey-island && useradd --no-log-init -r -g monkey-island monkey-island RUN chmod 444 /monkey/monkey_island/cc/server.key RUN chmod 444 /monkey/monkey_island/cc/server.csr diff --git a/monkey/monkey_island/cc/setup/data_dir.py b/monkey/monkey_island/cc/setup/data_dir.py index e2697d5d5..72a99b59f 100644 --- a/monkey/monkey_island/cc/setup/data_dir.py +++ b/monkey/monkey_island/cc/setup/data_dir.py @@ -5,6 +5,7 @@ from pathlib import Path from common.version import get_version from monkey_island.cc.server_utils.file_utils import create_secure_directory +from monkey_island.cc.setup.env_utils import is_running_on_docker from monkey_island.cc.setup.version_file_setup import get_version_from_dir, write_version logger = logging.getLogger(__name__) @@ -26,6 +27,17 @@ def setup_data_dir(data_dir_path: Path) -> None: def _is_data_dir_old(data_dir_path: Path) -> bool: dir_exists = data_dir_path.exists() + + if is_running_on_docker(): + if _data_dir_version_mismatch_exists(data_dir_path): + error_message = "Found an old volume. " + "You must create an empty volume for each docker container " + "as specified in setup documentation: " + "https://www.guardicore.com/infectionmonkey/docs/setup/docker/" + raise IncompatibleDataDirectory(error_message) + else: + return False + if not dir_exists or not os.listdir(data_dir_path): return False diff --git a/monkey/monkey_island/cc/setup/env_utils.py b/monkey/monkey_island/cc/setup/env_utils.py new file mode 100644 index 000000000..07f6417ea --- /dev/null +++ b/monkey/monkey_island/cc/setup/env_utils.py @@ -0,0 +1,8 @@ +import os + +# Must match evn var name in build_scripts/docker/Dockerfile:21 +DOCKER_ENV_VAR = "MONKEY_DOCKER_CONTAINER" + + +def is_running_on_docker(): + return os.environ.get(DOCKER_ENV_VAR) == "true" diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py index 34912ef50..fd1dbb693 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py @@ -3,6 +3,7 @@ from pathlib import Path import pytest from monkey_island.cc.setup.data_dir import IncompatibleDataDirectory, setup_data_dir +from monkey_island.cc.setup.env_utils import DOCKER_ENV_VAR from monkey_island.cc.setup.version_file_setup import _version_filename current_version = "1.1.1" @@ -27,6 +28,12 @@ def temp_version_file_path(temp_data_dir_path) -> Path: return temp_data_dir_path / _version_filename +def create_bogus_file(dir_path: Path) -> Path: + bogus_file_path = dir_path / "test.txt" + bogus_file_path.touch() + return bogus_file_path + + def test_setup_data_dir(temp_data_dir_path, temp_version_file_path): data_dir_path = temp_data_dir_path setup_data_dir(data_dir_path) @@ -41,8 +48,7 @@ def test_old_version_removed(monkeypatch, temp_data_dir_path, temp_version_file_ temp_data_dir_path.mkdir() temp_version_file_path.write_text(old_version) - bogus_file_path = temp_data_dir_path / "test.txt" - bogus_file_path.touch() + bogus_file_path = create_bogus_file(temp_data_dir_path) setup_data_dir(temp_data_dir_path) @@ -58,8 +64,7 @@ def test_old_version_not_removed( temp_data_dir_path.mkdir() temp_version_file_path.write_text(old_version) - bogus_file_path = temp_data_dir_path / "test.txt" - bogus_file_path.touch() + bogus_file_path = create_bogus_file(temp_data_dir_path) with pytest.raises(IncompatibleDataDirectory): setup_data_dir(temp_data_dir_path) @@ -71,8 +76,7 @@ def test_old_version_not_removed( def test_data_dir_setup_not_needed(temp_data_dir_path, temp_version_file_path): temp_data_dir_path.mkdir() temp_version_file_path.write_text(current_version) - bogus_file_path = temp_data_dir_path / "test.txt" - bogus_file_path.touch() + bogus_file_path = create_bogus_file(temp_data_dir_path) setup_data_dir(temp_data_dir_path) assert temp_version_file_path.read_text() == current_version @@ -84,3 +88,25 @@ def test_empty_data_dir(temp_data_dir_path, temp_version_file_path): setup_data_dir(temp_data_dir_path) assert temp_version_file_path.read_text() == current_version + + +def test_new_data_dir_docker(monkeypatch, temp_data_dir_path, temp_version_file_path): + monkeypatch.setenv(DOCKER_ENV_VAR, "true") + + temp_data_dir_path.mkdir() + bogus_file_path = create_bogus_file(temp_data_dir_path) + temp_version_file_path.write_text(current_version) + + setup_data_dir(temp_data_dir_path) + assert temp_version_file_path.read_text() == current_version + assert bogus_file_path.is_file() + + +def test_old_data_dir_docker(monkeypatch, temp_data_dir_path, temp_version_file_path): + monkeypatch.setenv(DOCKER_ENV_VAR, "true") + + temp_data_dir_path.mkdir() + temp_version_file_path.write_text(old_version) + + with pytest.raises(IncompatibleDataDirectory): + setup_data_dir(temp_data_dir_path) From 9ef9ba002452d43eda4cb8ab61dc244977307f3a Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Mon, 25 Oct 2021 18:28:44 +0300 Subject: [PATCH 2/3] Island: improve and fix data directory exception handling/logging --- monkey/monkey_island/cc/server_setup.py | 3 ++- monkey/monkey_island/cc/setup/data_dir.py | 33 +++++++++++++++-------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/monkey/monkey_island/cc/server_setup.py b/monkey/monkey_island/cc/server_setup.py index dd5547659..c6dc9c0b9 100644 --- a/monkey/monkey_island/cc/server_setup.py +++ b/monkey/monkey_island/cc/server_setup.py @@ -70,7 +70,8 @@ def _setup_data_dir(island_args: IslandCmdArgs) -> Tuple[IslandConfigOptions, st except json.JSONDecodeError as ex: print(f"Error loading server config: {ex}") exit(1) - except IncompatibleDataDirectory: + except IncompatibleDataDirectory as ex: + print(f"Incompatible data directory: {ex}") exit(1) diff --git a/monkey/monkey_island/cc/setup/data_dir.py b/monkey/monkey_island/cc/setup/data_dir.py index 72a99b59f..af01da050 100644 --- a/monkey/monkey_island/cc/setup/data_dir.py +++ b/monkey/monkey_island/cc/setup/data_dir.py @@ -29,33 +29,44 @@ def _is_data_dir_old(data_dir_path: Path) -> bool: dir_exists = data_dir_path.exists() if is_running_on_docker(): - if _data_dir_version_mismatch_exists(data_dir_path): - error_message = "Found an old volume. " - "You must create an empty volume for each docker container " - "as specified in setup documentation: " - "https://www.guardicore.com/infectionmonkey/docs/setup/docker/" - raise IncompatibleDataDirectory(error_message) - else: - return False + return _is_docker_data_dir_old(data_dir_path) - if not dir_exists or not os.listdir(data_dir_path): + if not dir_exists or _is_directory_empty(data_dir_path): return False return _data_dir_version_mismatch_exists(data_dir_path) +def _is_docker_data_dir_old(data_dir_path: Path) -> bool: + if _data_dir_version_mismatch_exists(data_dir_path): + if _is_directory_empty(data_dir_path): + return False + else: + raise IncompatibleDataDirectory( + "Found an old volume. " + "You must create an empty volume for each docker container " + "as specified in setup documentation: " + "https://www.guardicore.com/infectionmonkey/docs/setup/docker/" + ) + else: + return False + + +def _is_directory_empty(path: Path) -> bool: + return not os.listdir(path) + + def _handle_old_data_directory(data_dir_path: Path) -> None: should_delete_data_directory = _prompt_user_to_delete_data_directory(data_dir_path) if should_delete_data_directory: shutil.rmtree(data_dir_path) logger.info(f"{data_dir_path} was deleted.") else: - logger.error( + raise IncompatibleDataDirectory( "Unable to set up data directory. Please backup and delete the existing data directory" f" ({data_dir_path}). Then, try again. To learn how to restore and use a backup, please" " refer to the documentation." ) - raise IncompatibleDataDirectory() def _prompt_user_to_delete_data_directory(data_dir_path: Path) -> bool: From 01f8488b0735e2c98e7383891c32670d4cc45753 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Mon, 25 Oct 2021 18:30:53 +0300 Subject: [PATCH 3/3] UT's: assert correct behavior on docker if empty data directory is present and if no version file, but other files are present in the data directory --- .../monkey_island/cc/setup/test_data_dir.py | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py index fd1dbb693..e476784b1 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py @@ -102,7 +102,7 @@ def test_new_data_dir_docker(monkeypatch, temp_data_dir_path, temp_version_file_ assert bogus_file_path.is_file() -def test_old_data_dir_docker(monkeypatch, temp_data_dir_path, temp_version_file_path): +def test_data_dir_docker_old_version(monkeypatch, temp_data_dir_path, temp_version_file_path): monkeypatch.setenv(DOCKER_ENV_VAR, "true") temp_data_dir_path.mkdir() @@ -110,3 +110,22 @@ def test_old_data_dir_docker(monkeypatch, temp_data_dir_path, temp_version_file_ with pytest.raises(IncompatibleDataDirectory): setup_data_dir(temp_data_dir_path) + + +def test_empty_data_dir_docker(monkeypatch, temp_data_dir_path, temp_version_file_path): + monkeypatch.setenv(DOCKER_ENV_VAR, "true") + + temp_data_dir_path.mkdir() + + setup_data_dir(temp_data_dir_path) + assert temp_version_file_path.read_text() == current_version + + +def test_old_data_dir_docker_no_version(monkeypatch, temp_data_dir_path): + monkeypatch.setenv(DOCKER_ENV_VAR, "true") + + temp_data_dir_path.mkdir() + create_bogus_file(temp_data_dir_path) + + with pytest.raises(IncompatibleDataDirectory): + setup_data_dir(temp_data_dir_path)