From 5bcd9176fc48b833d6680e1274c8ec45c04e09c9 Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 10 May 2021 12:23:50 +0530 Subject: [PATCH 01/20] Remove `island_logger_config.json\' --- appimage/island_logger_config.json | 33 ------------------------------ 1 file changed, 33 deletions(-) delete mode 100644 appimage/island_logger_config.json diff --git a/appimage/island_logger_config.json b/appimage/island_logger_config.json deleted file mode 100644 index 4acf875e3..000000000 --- a/appimage/island_logger_config.json +++ /dev/null @@ -1,33 +0,0 @@ -{ - "version": 1, - "disable_existing_loggers": false, - "formatters": { - "simple": { - "format": "%(asctime)s - %(filename)s:%(lineno)s - %(funcName)10s() - %(levelname)s - %(message)s" - } - }, - "handlers": { - "console": { - "class": "logging.StreamHandler", - "level": "DEBUG", - "formatter": "simple", - "stream": "ext://sys.stdout" - }, - "info_file_handler": { - "class": "logging.handlers.RotatingFileHandler", - "level": "INFO", - "formatter": "simple", - "filename": "~/.monkey_island/info.log", - "maxBytes": 10485760, - "backupCount": 20, - "encoding": "utf8" - } - }, - "root": { - "level": "DEBUG", - "handlers": [ - "console", - "info_file_handler" - ] - } -} From ab895903891b17ca95c15edd72f9b56a8efc0e99 Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 10 May 2021 13:05:06 +0530 Subject: [PATCH 02/20] Remove `--logger-config` command-line argument, add "log_level" to server_config.json --- monkey/monkey_island.py | 21 ++++++++++++++++++- monkey/monkey_island/cc/arg_parser.py | 14 ++----------- .../monkey_island/cc/server_utils/consts.py | 2 ++ 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/monkey/monkey_island.py b/monkey/monkey_island.py index d32019b6a..ca5b2639b 100644 --- a/monkey/monkey_island.py +++ b/monkey/monkey_island.py @@ -5,7 +5,11 @@ from monkey_island.cc.arg_parser import parse_cli_args gevent_monkey.patch_all() import json # noqa: E402 +import os # noqa: E402 +from pathlib import Path # noqa: E402 +import monkey_island.cc.environment.server_config_generator as server_config_generator # noqa: E402 +from monkey_island.cc.server_utils.consts import DEFAULT_DATA_DIR, DEFAULT_LOG_LEVEL # noqa: E402 from monkey_island.cc.server_utils.island_logger import json_setup_logging # noqa: E402 if "__main__" == __name__: @@ -14,7 +18,22 @@ if "__main__" == __name__: # This is here in order to catch EVERYTHING, some functions are being called on # imports, so the log init needs to be first. try: - json_setup_logging(island_args.logger_config) + server_config_path = os.path.expanduser(island_args.server_config) + if not Path(server_config_path).is_file(): + server_config_generator.create_default_config_file(server_config_path) + + with open(server_config_path, "r") as f: + config_content = f.read() + data = json.loads(config_content) + data_dir = os.path.abspath( + os.path.expanduser( + os.path.expandvars(data["data_dir"] if "data_dir" in data else DEFAULT_DATA_DIR) + ) + ) + log_level = data["log_level"] if "log_level" in data else DEFAULT_LOG_LEVEL + + # json_setup_logging(data_dir, log_level) + except json.JSONDecodeError as ex: print(f"Error loading logging config: {ex}") exit(1) diff --git a/monkey/monkey_island/cc/arg_parser.py b/monkey/monkey_island/cc/arg_parser.py index 91a2b7d25..6e12ef38f 100644 --- a/monkey/monkey_island/cc/arg_parser.py +++ b/monkey/monkey_island/cc/arg_parser.py @@ -1,16 +1,12 @@ from dataclasses import dataclass -from monkey_island.cc.server_utils.consts import ( - DEFAULT_LOGGER_CONFIG_PATH, - DEFAULT_SERVER_CONFIG_PATH, -) +from monkey_island.cc.server_utils.consts import DEFAULT_SERVER_CONFIG_PATH @dataclass class IslandArgs: setup_only: bool server_config: str - logger_config: str def parse_cli_args() -> IslandArgs: @@ -34,12 +30,6 @@ def parse_cli_args() -> IslandArgs: help="The path to the server configuration file.", default=DEFAULT_SERVER_CONFIG_PATH, ) - parser.add_argument( - "--logger-config", - action="store", - help="The path to the logging configuration file.", - default=DEFAULT_LOGGER_CONFIG_PATH, - ) args = parser.parse_args() - return IslandArgs(args.setup_only, args.server_config, args.logger_config) + return IslandArgs(args.setup_only, args.server_config) diff --git a/monkey/monkey_island/cc/server_utils/consts.py b/monkey/monkey_island/cc/server_utils/consts.py index 67c7209eb..41bd6e4a7 100644 --- a/monkey/monkey_island/cc/server_utils/consts.py +++ b/monkey/monkey_island/cc/server_utils/consts.py @@ -15,4 +15,6 @@ DEFAULT_LOGGER_CONFIG_PATH = os.path.join( MONKEY_ISLAND_ABS_PATH, "cc", "island_logger_default_config.json" ) +DEFAULT_LOG_LEVEL = "NOTSET" + DEFAULT_DATA_DIR = os.path.join(MONKEY_ISLAND_ABS_PATH, "cc") From 785f2ef77da311e3893de13ee9a5022a7bd7b60b Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 10 May 2021 13:49:33 +0530 Subject: [PATCH 03/20] Replace `json_setup_logging()` with `setup_logging()` to configure logger --- monkey/monkey_island.py | 4 +- .../cc/island_logger_default_config.json | 33 ---------- .../monkey_island/cc/server_utils/consts.py | 4 -- .../cc/server_utils/island_logger.py | 65 ++++++++++++------- 4 files changed, 45 insertions(+), 61 deletions(-) delete mode 100644 monkey/monkey_island/cc/island_logger_default_config.json diff --git a/monkey/monkey_island.py b/monkey/monkey_island.py index ca5b2639b..2d86603f5 100644 --- a/monkey/monkey_island.py +++ b/monkey/monkey_island.py @@ -10,7 +10,7 @@ from pathlib import Path # noqa: E402 import monkey_island.cc.environment.server_config_generator as server_config_generator # noqa: E402 from monkey_island.cc.server_utils.consts import DEFAULT_DATA_DIR, DEFAULT_LOG_LEVEL # noqa: E402 -from monkey_island.cc.server_utils.island_logger import json_setup_logging # noqa: E402 +from monkey_island.cc.server_utils.island_logger import setup_logging # noqa: E402 if "__main__" == __name__: island_args = parse_cli_args() @@ -32,7 +32,7 @@ if "__main__" == __name__: ) log_level = data["log_level"] if "log_level" in data else DEFAULT_LOG_LEVEL - # json_setup_logging(data_dir, log_level) + setup_logging(data_dir, log_level) except json.JSONDecodeError as ex: print(f"Error loading logging config: {ex}") diff --git a/monkey/monkey_island/cc/island_logger_default_config.json b/monkey/monkey_island/cc/island_logger_default_config.json deleted file mode 100644 index 522177cda..000000000 --- a/monkey/monkey_island/cc/island_logger_default_config.json +++ /dev/null @@ -1,33 +0,0 @@ -{ - "version": 1, - "disable_existing_loggers": false, - "formatters": { - "simple": { - "format": "%(asctime)s - %(filename)s:%(lineno)s - %(funcName)10s() - %(levelname)s - %(message)s" - } - }, - "handlers": { - "console": { - "class": "logging.StreamHandler", - "level": "DEBUG", - "formatter": "simple", - "stream": "ext://sys.stdout" - }, - "info_file_handler": { - "class": "logging.handlers.RotatingFileHandler", - "level": "INFO", - "formatter": "simple", - "filename": "info.log", - "maxBytes": 10485760, - "backupCount": 20, - "encoding": "utf8" - } - }, - "root": { - "level": "DEBUG", - "handlers": [ - "console", - "info_file_handler" - ] - } -} \ No newline at end of file diff --git a/monkey/monkey_island/cc/server_utils/consts.py b/monkey/monkey_island/cc/server_utils/consts.py index 41bd6e4a7..afb0d5c89 100644 --- a/monkey/monkey_island/cc/server_utils/consts.py +++ b/monkey/monkey_island/cc/server_utils/consts.py @@ -11,10 +11,6 @@ DEFAULT_DEVELOP_SERVER_CONFIG_PATH = os.path.join( MONKEY_ISLAND_ABS_PATH, "cc", "server_config.json.develop" ) -DEFAULT_LOGGER_CONFIG_PATH = os.path.join( - MONKEY_ISLAND_ABS_PATH, "cc", "island_logger_default_config.json" -) - DEFAULT_LOG_LEVEL = "NOTSET" DEFAULT_DATA_DIR = os.path.join(MONKEY_ISLAND_ABS_PATH, "cc") diff --git a/monkey/monkey_island/cc/server_utils/island_logger.py b/monkey/monkey_island/cc/server_utils/island_logger.py index a32f6505f..4405db5e1 100644 --- a/monkey/monkey_island/cc/server_utils/island_logger.py +++ b/monkey/monkey_island/cc/server_utils/island_logger.py @@ -1,38 +1,59 @@ -import json import logging.config import os +from copy import deepcopy from typing import Dict -from monkey_island.cc.server_utils.consts import DEFAULT_LOGGER_CONFIG_PATH - __author__ = "Maor.Rayzin" -def json_setup_logging( - default_path=DEFAULT_LOGGER_CONFIG_PATH, - default_level=logging.INFO, - env_key="LOG_CFG", +LOGGER_CONFIG_DICT = { + "version": 1, + "disable_existing_loggers": False, + "formatters": { + "simple": { + "format": "%(asctime)s - %(filename)s:%(lineno)s - " + + "%(funcName)10s() - %(levelname)s - %(message)s" + } + }, + "handlers": { + "console": { + "class": "logging.StreamHandler", + "level": "DEBUG", + "formatter": "simple", + "stream": "ext://sys.stdout", + }, + "info_file_handler": { + "class": "logging.handlers.RotatingFileHandler", + "level": "INFO", + "formatter": "simple", + "filename": None, # set in setup_logging() + "maxBytes": 10485760, + "backupCount": 20, + "encoding": "utf8", + }, + }, + "root": {"level": None, "handlers": ["console", "info_file_handler"]}, # set in setup_logging() +} + + +def setup_logging( + data_dir_path, + log_level, ): """ Setup the logging configuration - :param default_path: the default log configuration file path - :param default_level: Default level to log from - :param env_key: SYS ENV key to use for external configuration file path + :param data_dir_path: data directory file path + :param log_level: level to log from :return: """ - path = os.path.expanduser(default_path) - value = os.getenv(env_key, None) - if value: - path = value - - if os.path.exists(path): - with open(path, "rt") as f: - config = json.load(f) - _expanduser_log_file_paths(config) - logging.config.dictConfig(config) - else: - logging.basicConfig(level=default_level) + logger_configuration = deepcopy(LOGGER_CONFIG_DICT) + _expanduser_log_file_paths(logger_configuration) + logger_configuration["root"]["level"] = log_level + logger_configuration["handlers"]["info_file_handler"]["filename"] = os.path.join( + data_dir_path, "monkey_island.log" + ) + logging.config.dictConfig(logger_configuration) def _expanduser_log_file_paths(config: Dict): From f84e4aed2c89e97e86cc5eb4668fcd7006ee3e36 Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 10 May 2021 14:04:07 +0530 Subject: [PATCH 04/20] Set log filename in config before expanding its paths --- monkey/monkey_island/cc/server_utils/island_logger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monkey/monkey_island/cc/server_utils/island_logger.py b/monkey/monkey_island/cc/server_utils/island_logger.py index 4405db5e1..70a754810 100644 --- a/monkey/monkey_island/cc/server_utils/island_logger.py +++ b/monkey/monkey_island/cc/server_utils/island_logger.py @@ -48,11 +48,11 @@ def setup_logging( """ logger_configuration = deepcopy(LOGGER_CONFIG_DICT) - _expanduser_log_file_paths(logger_configuration) logger_configuration["root"]["level"] = log_level logger_configuration["handlers"]["info_file_handler"]["filename"] = os.path.join( data_dir_path, "monkey_island.log" ) + _expanduser_log_file_paths(logger_configuration) logging.config.dictConfig(logger_configuration) From 6d04e7cbb4951a74fcfa697f138d3d94ca2a9ded Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 10 May 2021 14:52:07 +0530 Subject: [PATCH 05/20] Fix unit tests and modify code based on failed tests (tests/monkey_island/cc/server_utils/test_island_logger.py) --- .../cc/server_utils/island_logger.py | 12 +++++++----- .../cc/server_utils/test_island_logger.py | 15 ++++++--------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/island_logger.py b/monkey/monkey_island/cc/server_utils/island_logger.py index 70a754810..dd00cb0d7 100644 --- a/monkey/monkey_island/cc/server_utils/island_logger.py +++ b/monkey/monkey_island/cc/server_utils/island_logger.py @@ -36,10 +36,7 @@ LOGGER_CONFIG_DICT = { } -def setup_logging( - data_dir_path, - log_level, -): +def setup_logging(data_dir_path, log_level): """ Setup the logging configuration :param data_dir_path: data directory file path @@ -48,11 +45,16 @@ def setup_logging( """ logger_configuration = deepcopy(LOGGER_CONFIG_DICT) - logger_configuration["root"]["level"] = log_level + + if not os.path.exists(data_dir_path): + os.makedirs(data_dir_path, mode=0o700, exist_ok=True) + logger_configuration["handlers"]["info_file_handler"]["filename"] = os.path.join( data_dir_path, "monkey_island.log" ) + logger_configuration["root"]["level"] = log_level _expanduser_log_file_paths(logger_configuration) + logging.config.dictConfig(logger_configuration) diff --git a/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py b/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py index 57f0cc5b0..3dd7e9570 100644 --- a/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py +++ b/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py @@ -3,12 +3,7 @@ import os import pytest -from monkey_island.cc.server_utils.island_logger import json_setup_logging - - -@pytest.fixture() -def test_logger_config_path(resources_dir): - return os.path.join(resources_dir, "logger_config.json") +from monkey_island.cc.server_utils.island_logger import setup_logging # TODO move into monkey/monkey_island/cc/test_common/fixtures after rebase/backmerge @@ -17,11 +12,13 @@ def mock_home_env(monkeypatch, tmpdir): monkeypatch.setenv("HOME", str(tmpdir)) -def test_expanduser_filename(mock_home_env, tmpdir, test_logger_config_path): - INFO_LOG = os.path.join(tmpdir, "info.log") +def test_expanduser_filename(mock_home_env, tmpdir): + DATA_DIR = tmpdir + INFO_LOG = os.path.join(DATA_DIR, "monkey_island.log") + LOG_LEVEL = "DEBUG" TEST_STRING = "Hello, Monkey!" - json_setup_logging(test_logger_config_path) + setup_logging(DATA_DIR, LOG_LEVEL) logger = logging.getLogger("TestLogger") logger.info(TEST_STRING) From 0556465c6ad2b3b975782decca9c27d6cad393bb Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 10 May 2021 16:47:30 +0530 Subject: [PATCH 06/20] Update CHANGELOG.md (removed island logger config) --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2150b874..8c3c38459 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). instead of $HOME. #1143 - Authentication mechanism to use bcrypt on server side. #1139 - Removed relevant dead code as reported by Vulture. #1149 +- Removed island logger config and added "log_level" to server config. #1151 ### Fixed - Attempted to delete a directory when monkey config reset was called. #1054 From 8dc84ee0f75bee6cecc0484b509b615b46218d38 Mon Sep 17 00:00:00 2001 From: Shreya Date: Tue, 11 May 2021 17:56:49 +0530 Subject: [PATCH 07/20] Assume configured data directory exists when configuring the logger --- monkey/monkey_island/cc/server_utils/island_logger.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/island_logger.py b/monkey/monkey_island/cc/server_utils/island_logger.py index dd00cb0d7..abc0bba9d 100644 --- a/monkey/monkey_island/cc/server_utils/island_logger.py +++ b/monkey/monkey_island/cc/server_utils/island_logger.py @@ -46,9 +46,6 @@ def setup_logging(data_dir_path, log_level): logger_configuration = deepcopy(LOGGER_CONFIG_DICT) - if not os.path.exists(data_dir_path): - os.makedirs(data_dir_path, mode=0o700, exist_ok=True) - logger_configuration["handlers"]["info_file_handler"]["filename"] = os.path.join( data_dir_path, "monkey_island.log" ) From 805ab989b99cc00721700bca663be324f3528042 Mon Sep 17 00:00:00 2001 From: Shreya Date: Tue, 11 May 2021 17:58:07 +0530 Subject: [PATCH 08/20] Remove "__author__" field --- monkey/monkey_island/cc/server_utils/island_logger.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/island_logger.py b/monkey/monkey_island/cc/server_utils/island_logger.py index abc0bba9d..dff77f628 100644 --- a/monkey/monkey_island/cc/server_utils/island_logger.py +++ b/monkey/monkey_island/cc/server_utils/island_logger.py @@ -3,9 +3,6 @@ import os from copy import deepcopy from typing import Dict -__author__ = "Maor.Rayzin" - - LOGGER_CONFIG_DICT = { "version": 1, "disable_existing_loggers": False, From 3c7687a40537f1d118f5a34794cf3a38a14fc77d Mon Sep 17 00:00:00 2001 From: Shreya Date: Tue, 11 May 2021 18:08:07 +0530 Subject: [PATCH 09/20] Catch and print errors instead of creating a default server config --- monkey/monkey_island.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/monkey/monkey_island.py b/monkey/monkey_island.py index 2d86603f5..64d4b1f83 100644 --- a/monkey/monkey_island.py +++ b/monkey/monkey_island.py @@ -6,9 +6,7 @@ gevent_monkey.patch_all() import json # noqa: E402 import os # noqa: E402 -from pathlib import Path # noqa: E402 -import monkey_island.cc.environment.server_config_generator as server_config_generator # noqa: E402 from monkey_island.cc.server_utils.consts import DEFAULT_DATA_DIR, DEFAULT_LOG_LEVEL # noqa: E402 from monkey_island.cc.server_utils.island_logger import setup_logging # noqa: E402 @@ -19,9 +17,6 @@ if "__main__" == __name__: # imports, so the log init needs to be first. try: server_config_path = os.path.expanduser(island_args.server_config) - if not Path(server_config_path).is_file(): - server_config_generator.create_default_config_file(server_config_path) - with open(server_config_path, "r") as f: config_content = f.read() data = json.loads(config_content) @@ -34,8 +29,12 @@ if "__main__" == __name__: setup_logging(data_dir, log_level) + except OSError as ex: + print(f"Error opening server config file: {ex}") + exit(1) + except json.JSONDecodeError as ex: - print(f"Error loading logging config: {ex}") + print(f"Error loading server config: {ex}") exit(1) from monkey_island.cc.main import main # noqa: E402 From e8c1c81edf6b3ddffdec0771ad0fd6d02e6ef5ac Mon Sep 17 00:00:00 2001 From: Shreya Date: Tue, 11 May 2021 18:16:45 +0530 Subject: [PATCH 10/20] Move `DEFAULT_LOG_LEVEL` and add function `load_server_config` to monkey_island.py --- monkey/monkey_island.py | 30 ++++++++++++------- .../monkey_island/cc/server_utils/consts.py | 2 -- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/monkey/monkey_island.py b/monkey/monkey_island.py index 64d4b1f83..b0e510d21 100644 --- a/monkey/monkey_island.py +++ b/monkey/monkey_island.py @@ -7,9 +7,26 @@ gevent_monkey.patch_all() import json # noqa: E402 import os # noqa: E402 -from monkey_island.cc.server_utils.consts import DEFAULT_DATA_DIR, DEFAULT_LOG_LEVEL # noqa: E402 +from monkey_island.cc.server_utils.consts import DEFAULT_DATA_DIR # noqa: E402 from monkey_island.cc.server_utils.island_logger import setup_logging # noqa: E402 +DEFAULT_LOG_LEVEL = "INFO" + + +def load_server_config(path): + with open(server_config_path, "r") as f: + config_content = f.read() + data = json.loads(config_content) + data_dir = os.path.abspath( + os.path.expanduser( + os.path.expandvars(data["data_dir"] if "data_dir" in data else DEFAULT_DATA_DIR) + ) + ) + log_level = data["log_level"] if "log_level" in data else DEFAULT_LOG_LEVEL + + return data_dir, log_level + + if "__main__" == __name__: island_args = parse_cli_args() @@ -17,15 +34,8 @@ if "__main__" == __name__: # imports, so the log init needs to be first. try: server_config_path = os.path.expanduser(island_args.server_config) - with open(server_config_path, "r") as f: - config_content = f.read() - data = json.loads(config_content) - data_dir = os.path.abspath( - os.path.expanduser( - os.path.expandvars(data["data_dir"] if "data_dir" in data else DEFAULT_DATA_DIR) - ) - ) - log_level = data["log_level"] if "log_level" in data else DEFAULT_LOG_LEVEL + + data_dir, log_level = load_server_config(server_config_path) setup_logging(data_dir, log_level) diff --git a/monkey/monkey_island/cc/server_utils/consts.py b/monkey/monkey_island/cc/server_utils/consts.py index afb0d5c89..f0dba26dc 100644 --- a/monkey/monkey_island/cc/server_utils/consts.py +++ b/monkey/monkey_island/cc/server_utils/consts.py @@ -11,6 +11,4 @@ DEFAULT_DEVELOP_SERVER_CONFIG_PATH = os.path.join( MONKEY_ISLAND_ABS_PATH, "cc", "server_config.json.develop" ) -DEFAULT_LOG_LEVEL = "NOTSET" - DEFAULT_DATA_DIR = os.path.join(MONKEY_ISLAND_ABS_PATH, "cc") From c5ba48db53842e0c713b3a67c98fdd4f0844bdf1 Mon Sep 17 00:00:00 2001 From: Shreya Date: Tue, 11 May 2021 18:41:56 +0530 Subject: [PATCH 11/20] Modify/add unit tests (test_island_logger.py) --- .../cc/server_utils/island_logger.py | 27 +++++-------- .../cc/server_utils/test_island_logger.py | 39 ++++++++++++------- 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/monkey/monkey_island/cc/server_utils/island_logger.py b/monkey/monkey_island/cc/server_utils/island_logger.py index dff77f628..1e8b9b86e 100644 --- a/monkey/monkey_island/cc/server_utils/island_logger.py +++ b/monkey/monkey_island/cc/server_utils/island_logger.py @@ -1,7 +1,8 @@ import logging.config import os from copy import deepcopy -from typing import Dict + +ISLAND_LOG_FILENAME = "monkey_island.log" LOGGER_CONFIG_DICT = { "version": 1, @@ -13,15 +14,13 @@ LOGGER_CONFIG_DICT = { } }, "handlers": { - "console": { + "console_handler": { "class": "logging.StreamHandler", - "level": "DEBUG", "formatter": "simple", "stream": "ext://sys.stdout", }, - "info_file_handler": { + "file_handler": { "class": "logging.handlers.RotatingFileHandler", - "level": "INFO", "formatter": "simple", "filename": None, # set in setup_logging() "maxBytes": 10485760, @@ -29,7 +28,10 @@ LOGGER_CONFIG_DICT = { "encoding": "utf8", }, }, - "root": {"level": None, "handlers": ["console", "info_file_handler"]}, # set in setup_logging() + "root": { + "level": None, # set in setup_logging() + "handlers": ["console_handler", "file_handler"], + }, } @@ -43,18 +45,9 @@ def setup_logging(data_dir_path, log_level): logger_configuration = deepcopy(LOGGER_CONFIG_DICT) - logger_configuration["handlers"]["info_file_handler"]["filename"] = os.path.join( - data_dir_path, "monkey_island.log" + logger_configuration["handlers"]["file_handler"]["filename"] = os.path.join( + data_dir_path, ISLAND_LOG_FILENAME ) logger_configuration["root"]["level"] = log_level - _expanduser_log_file_paths(logger_configuration) logging.config.dictConfig(logger_configuration) - - -def _expanduser_log_file_paths(config: Dict): - handlers = config.get("handlers", {}) - - for handler_settings in handlers.values(): - if "filename" in handler_settings: - handler_settings["filename"] = os.path.expanduser(handler_settings["filename"]) diff --git a/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py b/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py index 3dd7e9570..9bcdf71f1 100644 --- a/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py +++ b/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py @@ -1,29 +1,38 @@ import logging import os -import pytest - -from monkey_island.cc.server_utils.island_logger import setup_logging +from monkey_island.cc.server_utils.island_logger import ISLAND_LOG_FILENAME, setup_logging -# TODO move into monkey/monkey_island/cc/test_common/fixtures after rebase/backmerge -@pytest.fixture -def mock_home_env(monkeypatch, tmpdir): - monkeypatch.setenv("HOME", str(tmpdir)) - - -def test_expanduser_filename(mock_home_env, tmpdir): +def test_setup_logging_log_level_debug(tmpdir): DATA_DIR = tmpdir - INFO_LOG = os.path.join(DATA_DIR, "monkey_island.log") + LOG_FILE = os.path.join(DATA_DIR, ISLAND_LOG_FILENAME) LOG_LEVEL = "DEBUG" - TEST_STRING = "Hello, Monkey!" + TEST_STRING = "Hello, Monkey! (Log level: debug)" setup_logging(DATA_DIR, LOG_LEVEL) logger = logging.getLogger("TestLogger") - logger.info(TEST_STRING) + logger.debug(TEST_STRING) - assert os.path.isfile(INFO_LOG) - with open(INFO_LOG, "r") as f: + assert os.path.isfile(LOG_FILE) + with open(LOG_FILE, "r") as f: line = f.readline() assert TEST_STRING in line + + +def test_setup_logging_log_level_info(tmpdir): + DATA_DIR = tmpdir + LOG_FILE = os.path.join(DATA_DIR, ISLAND_LOG_FILENAME) + LOG_LEVEL = "INFO" + TEST_STRING = "Hello, Monkey! (Log level: info)" + + setup_logging(DATA_DIR, LOG_LEVEL) + + logger = logging.getLogger("TestLogger") + logger.debug(TEST_STRING) + + assert os.path.isfile(LOG_FILE) + with open(LOG_FILE, "r") as f: + line = f.readline() + assert TEST_STRING not in line From 5f8145e3d1ead46def801b608b51cf953456e3d7 Mon Sep 17 00:00:00 2001 From: Shreya Date: Tue, 11 May 2021 19:06:38 +0530 Subject: [PATCH 12/20] Add tests for console logging (test_island_logger.py) --- .../cc/server_utils/test_island_logger.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py b/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py index 9bcdf71f1..241d7b584 100644 --- a/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py +++ b/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py @@ -36,3 +36,31 @@ def test_setup_logging_log_level_info(tmpdir): with open(LOG_FILE, "r") as f: line = f.readline() assert TEST_STRING not in line + + +def test_setup_logging_console_log_level_debug(capsys, tmpdir): + DATA_DIR = tmpdir + LOG_LEVEL = "DEBUG" + TEST_STRING = "Hello, Monkey! (Log level: debug)" + + setup_logging(DATA_DIR, LOG_LEVEL) + + logger = logging.getLogger("TestLogger") + logger.debug(TEST_STRING) + + captured = capsys.readouterr() + assert TEST_STRING in captured.out + + +def test_setup_logging_console_log_level_info(capsys, tmpdir): + DATA_DIR = tmpdir + LOG_LEVEL = "INFO" + TEST_STRING = "Hello, Monkey! (Log level: info)" + + setup_logging(DATA_DIR, LOG_LEVEL) + + logger = logging.getLogger("TestLogger") + logger.debug(TEST_STRING) + + captured = capsys.readouterr() + assert TEST_STRING not in captured.out From 83a235bb5d997bdaacea1716a3d08b2236e177f6 Mon Sep 17 00:00:00 2001 From: Shreya Date: Tue, 11 May 2021 19:08:28 +0530 Subject: [PATCH 13/20] Rename unit test functions in test_island_logger.py --- .../cc/server_utils/test_island_logger.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py b/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py index 241d7b584..c301f3d5b 100644 --- a/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py +++ b/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py @@ -4,11 +4,11 @@ import os from monkey_island.cc.server_utils.island_logger import ISLAND_LOG_FILENAME, setup_logging -def test_setup_logging_log_level_debug(tmpdir): +def test_setup_logging_file_log_level_debug(tmpdir): DATA_DIR = tmpdir LOG_FILE = os.path.join(DATA_DIR, ISLAND_LOG_FILENAME) LOG_LEVEL = "DEBUG" - TEST_STRING = "Hello, Monkey! (Log level: debug)" + TEST_STRING = "Hello, Monkey! (File; Log level: debug)" setup_logging(DATA_DIR, LOG_LEVEL) @@ -21,11 +21,11 @@ def test_setup_logging_log_level_debug(tmpdir): assert TEST_STRING in line -def test_setup_logging_log_level_info(tmpdir): +def test_setup_logging_file_log_level_info(tmpdir): DATA_DIR = tmpdir LOG_FILE = os.path.join(DATA_DIR, ISLAND_LOG_FILENAME) LOG_LEVEL = "INFO" - TEST_STRING = "Hello, Monkey! (Log level: info)" + TEST_STRING = "Hello, Monkey! (File; Log level: info)" setup_logging(DATA_DIR, LOG_LEVEL) @@ -41,7 +41,7 @@ def test_setup_logging_log_level_info(tmpdir): def test_setup_logging_console_log_level_debug(capsys, tmpdir): DATA_DIR = tmpdir LOG_LEVEL = "DEBUG" - TEST_STRING = "Hello, Monkey! (Log level: debug)" + TEST_STRING = "Hello, Monkey! (Console; Log level: debug)" setup_logging(DATA_DIR, LOG_LEVEL) @@ -55,7 +55,7 @@ def test_setup_logging_console_log_level_debug(capsys, tmpdir): def test_setup_logging_console_log_level_info(capsys, tmpdir): DATA_DIR = tmpdir LOG_LEVEL = "INFO" - TEST_STRING = "Hello, Monkey! (Log level: info)" + TEST_STRING = "Hello, Monkey! (Console; Log level: info)" setup_logging(DATA_DIR, LOG_LEVEL) From f7bef76f39beb9daeab6f6f34acbf434d07f8d5a Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 11 May 2021 13:00:44 -0400 Subject: [PATCH 14/20] island: Move load_server_config() to config_loader.py Because `monkey_island.py` has the same name as the `monkey_island` module, pytest can't import monkey_island.py and run any tests against its code. --- monkey/monkey_island.py | 21 ++------------------- monkey/monkey_island/config_loader.py | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 19 deletions(-) create mode 100644 monkey/monkey_island/config_loader.py diff --git a/monkey/monkey_island.py b/monkey/monkey_island.py index b0e510d21..8a2e0608b 100644 --- a/monkey/monkey_island.py +++ b/monkey/monkey_island.py @@ -7,26 +7,9 @@ gevent_monkey.patch_all() import json # noqa: E402 import os # noqa: E402 -from monkey_island.cc.server_utils.consts import DEFAULT_DATA_DIR # noqa: E402 +from monkey_island import config_loader # noqa: E402 from monkey_island.cc.server_utils.island_logger import setup_logging # noqa: E402 -DEFAULT_LOG_LEVEL = "INFO" - - -def load_server_config(path): - with open(server_config_path, "r") as f: - config_content = f.read() - data = json.loads(config_content) - data_dir = os.path.abspath( - os.path.expanduser( - os.path.expandvars(data["data_dir"] if "data_dir" in data else DEFAULT_DATA_DIR) - ) - ) - log_level = data["log_level"] if "log_level" in data else DEFAULT_LOG_LEVEL - - return data_dir, log_level - - if "__main__" == __name__: island_args = parse_cli_args() @@ -35,7 +18,7 @@ if "__main__" == __name__: try: server_config_path = os.path.expanduser(island_args.server_config) - data_dir, log_level = load_server_config(server_config_path) + data_dir, log_level = config_loader.load_server_config(server_config_path) setup_logging(data_dir, log_level) diff --git a/monkey/monkey_island/config_loader.py b/monkey/monkey_island/config_loader.py new file mode 100644 index 000000000..8f0a7d19b --- /dev/null +++ b/monkey/monkey_island/config_loader.py @@ -0,0 +1,20 @@ +import json +import os + +from monkey_island.cc.server_utils.consts import DEFAULT_DATA_DIR + +DEFAULT_LOG_LEVEL = "INFO" + + +def load_server_config(server_config_path): + with open(server_config_path, "r") as f: + config_content = f.read() + data = json.loads(config_content) + data_dir = os.path.abspath( + os.path.expanduser( + os.path.expandvars(data["data_dir"] if "data_dir" in data else DEFAULT_DATA_DIR) + ) + ) + log_level = data["log_level"] if "log_level" in data else DEFAULT_LOG_LEVEL + + return data_dir, log_level From 5847674d9209122bf42f9259d08b78d8cf9dd5e4 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 11 May 2021 13:14:07 -0400 Subject: [PATCH 15/20] island: Add unit test for load_server_config() --- monkey/tests/conftest.py | 17 +++++++++++++++++ .../tests/monkey_island/test_config_loader.py | 10 ++++++++++ .../server_configs/test_server_config.json | 4 ++++ 3 files changed, 31 insertions(+) create mode 100644 monkey/tests/monkey_island/test_config_loader.py create mode 100644 monkey/tests/resources/server_configs/test_server_config.json diff --git a/monkey/tests/conftest.py b/monkey/tests/conftest.py index 81806ef69..328cb109c 100644 --- a/monkey/tests/conftest.py +++ b/monkey/tests/conftest.py @@ -46,3 +46,20 @@ def with_data_dir(environment_resources_dir): @pytest.fixture(scope="session") def with_data_dir_home(environment_resources_dir): return os.path.join(environment_resources_dir, "server_config_with_data_dir_home.json") + + +@pytest.fixture(scope="session") +def server_config_resources_dir(resources_dir): + return os.path.join(resources_dir, "server_configs") + + +@pytest.fixture(scope="session") +def test_server_config(server_config_resources_dir): + return os.path.join(server_config_resources_dir, "test_server_config.json") + + +@pytest.fixture +def mock_home_env(monkeypatch, tmpdir): + monkeypatch.setenv("HOME", str(tmpdir)) + + return tmpdir diff --git a/monkey/tests/monkey_island/test_config_loader.py b/monkey/tests/monkey_island/test_config_loader.py new file mode 100644 index 000000000..ac2e35377 --- /dev/null +++ b/monkey/tests/monkey_island/test_config_loader.py @@ -0,0 +1,10 @@ +import os + +from monkey_island import config_loader + + +def test_load_server_config_from_file(test_server_config, mock_home_env): + (data_dir, log_level) = config_loader.load_server_config(test_server_config) + + assert data_dir == os.path.join(mock_home_env, ".monkey_island") + assert log_level == "NOTICE" diff --git a/monkey/tests/resources/server_configs/test_server_config.json b/monkey/tests/resources/server_configs/test_server_config.json new file mode 100644 index 000000000..25082b0b3 --- /dev/null +++ b/monkey/tests/resources/server_configs/test_server_config.json @@ -0,0 +1,4 @@ +{ + "data_dir": "~/.monkey_island", + "log_level": "NOTICE" +} From 990244c3ac7605457b9253ce1f5e7d61a7377c7b Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 11 May 2021 13:16:24 -0400 Subject: [PATCH 16/20] island: Return config dict from load_server_config() As the number of configuration items will increase in the future, return the config dict instead of individual config properties. --- monkey/monkey_island.py | 4 ++-- monkey/monkey_island/config_loader.py | 10 +++++----- monkey/tests/monkey_island/test_config_loader.py | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/monkey/monkey_island.py b/monkey/monkey_island.py index 8a2e0608b..51f2e1a11 100644 --- a/monkey/monkey_island.py +++ b/monkey/monkey_island.py @@ -18,9 +18,9 @@ if "__main__" == __name__: try: server_config_path = os.path.expanduser(island_args.server_config) - data_dir, log_level = config_loader.load_server_config(server_config_path) + config = config_loader.load_server_config(server_config_path) - setup_logging(data_dir, log_level) + setup_logging(config["data_dir"], config["log_level"]) except OSError as ex: print(f"Error opening server config file: {ex}") diff --git a/monkey/monkey_island/config_loader.py b/monkey/monkey_island/config_loader.py index 8f0a7d19b..357cacbb8 100644 --- a/monkey/monkey_island/config_loader.py +++ b/monkey/monkey_island/config_loader.py @@ -9,12 +9,12 @@ DEFAULT_LOG_LEVEL = "INFO" def load_server_config(server_config_path): with open(server_config_path, "r") as f: config_content = f.read() - data = json.loads(config_content) - data_dir = os.path.abspath( + config = json.loads(config_content) + config["data_dir"] = os.path.abspath( os.path.expanduser( - os.path.expandvars(data["data_dir"] if "data_dir" in data else DEFAULT_DATA_DIR) + os.path.expandvars(config["data_dir"] if "data_dir" in config else DEFAULT_DATA_DIR) ) ) - log_level = data["log_level"] if "log_level" in data else DEFAULT_LOG_LEVEL + config["log_level"] = config["log_level"] if "log_level" in config else DEFAULT_LOG_LEVEL - return data_dir, log_level + return config diff --git a/monkey/tests/monkey_island/test_config_loader.py b/monkey/tests/monkey_island/test_config_loader.py index ac2e35377..f65bfe197 100644 --- a/monkey/tests/monkey_island/test_config_loader.py +++ b/monkey/tests/monkey_island/test_config_loader.py @@ -4,7 +4,7 @@ from monkey_island import config_loader def test_load_server_config_from_file(test_server_config, mock_home_env): - (data_dir, log_level) = config_loader.load_server_config(test_server_config) + config = config_loader.load_server_config(test_server_config) - assert data_dir == os.path.join(mock_home_env, ".monkey_island") - assert log_level == "NOTICE" + assert config["data_dir"] == os.path.join(mock_home_env, ".monkey_island") + assert config["log_level"] == "NOTICE" From de7865aa219f4da8d7359be073374d70af0264cb Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 11 May 2021 13:25:22 -0400 Subject: [PATCH 17/20] island: Add tests for default server config values --- monkey/monkey_island.py | 2 +- monkey/monkey_island/config_loader.py | 20 ++++++++++++------- .../tests/monkey_island/test_config_loader.py | 19 +++++++++++++++++- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/monkey/monkey_island.py b/monkey/monkey_island.py index 51f2e1a11..5363ac5de 100644 --- a/monkey/monkey_island.py +++ b/monkey/monkey_island.py @@ -18,7 +18,7 @@ if "__main__" == __name__: try: server_config_path = os.path.expanduser(island_args.server_config) - config = config_loader.load_server_config(server_config_path) + config = config_loader.load_server_config_from_file(server_config_path) setup_logging(config["data_dir"], config["log_level"]) diff --git a/monkey/monkey_island/config_loader.py b/monkey/monkey_island/config_loader.py index 357cacbb8..5cdb149b7 100644 --- a/monkey/monkey_island/config_loader.py +++ b/monkey/monkey_island/config_loader.py @@ -6,15 +6,21 @@ from monkey_island.cc.server_utils.consts import DEFAULT_DATA_DIR DEFAULT_LOG_LEVEL = "INFO" -def load_server_config(server_config_path): +def load_server_config_from_file(server_config_path): with open(server_config_path, "r") as f: config_content = f.read() config = json.loads(config_content) - config["data_dir"] = os.path.abspath( - os.path.expanduser( - os.path.expandvars(config["data_dir"] if "data_dir" in config else DEFAULT_DATA_DIR) - ) - ) - config["log_level"] = config["log_level"] if "log_level" in config else DEFAULT_LOG_LEVEL + add_default_values_to_config(config) return config + + +def add_default_values_to_config(config): + config["data_dir"] = os.path.abspath( + os.path.expanduser( + os.path.expandvars(config["data_dir"] if "data_dir" in config else DEFAULT_DATA_DIR) + ) + ) + config["log_level"] = config["log_level"] if "log_level" in config else DEFAULT_LOG_LEVEL + + return config diff --git a/monkey/tests/monkey_island/test_config_loader.py b/monkey/tests/monkey_island/test_config_loader.py index f65bfe197..20c330f6a 100644 --- a/monkey/tests/monkey_island/test_config_loader.py +++ b/monkey/tests/monkey_island/test_config_loader.py @@ -1,10 +1,27 @@ import os from monkey_island import config_loader +from monkey_island.cc.server_utils.consts import DEFAULT_DATA_DIR def test_load_server_config_from_file(test_server_config, mock_home_env): - config = config_loader.load_server_config(test_server_config) + config = config_loader.load_server_config_from_file(test_server_config) assert config["data_dir"] == os.path.join(mock_home_env, ".monkey_island") assert config["log_level"] == "NOTICE" + + +def test_default_log_level(): + test_config = {} + config = config_loader.add_default_values_to_config(test_config) + + assert "log_level" in config + assert config["log_level"] == "INFO" + + +def test_default_data_dir(mock_home_env): + test_config = {} + config = config_loader.add_default_values_to_config(test_config) + + assert "data_dir" in config + assert config["data_dir"] == DEFAULT_DATA_DIR From 5ea241f120e1276098f475db53bbf0efa2c43a8f Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 11 May 2021 13:29:53 -0400 Subject: [PATCH 18/20] island: Simplify logic in add_default_values_to_config() --- monkey/monkey_island/config_loader.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/monkey/monkey_island/config_loader.py b/monkey/monkey_island/config_loader.py index 5cdb149b7..aaa9185d7 100644 --- a/monkey/monkey_island/config_loader.py +++ b/monkey/monkey_island/config_loader.py @@ -17,10 +17,9 @@ def load_server_config_from_file(server_config_path): def add_default_values_to_config(config): config["data_dir"] = os.path.abspath( - os.path.expanduser( - os.path.expandvars(config["data_dir"] if "data_dir" in config else DEFAULT_DATA_DIR) - ) + os.path.expanduser(os.path.expandvars(config.get("data_dir", DEFAULT_DATA_DIR))) ) - config["log_level"] = config["log_level"] if "log_level" in config else DEFAULT_LOG_LEVEL + + config.setdefault("log_level", DEFAULT_LOG_LEVEL) return config From 08668f3eae0151079d2cb5383db1c02d90c28ba3 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 11 May 2021 13:40:58 -0400 Subject: [PATCH 19/20] island: Handle lower case log levels in server config --- .../monkey_island/cc/server_utils/island_logger.py | 2 +- .../cc/server_utils/test_island_logger.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/monkey/monkey_island/cc/server_utils/island_logger.py b/monkey/monkey_island/cc/server_utils/island_logger.py index 1e8b9b86e..c05915bc8 100644 --- a/monkey/monkey_island/cc/server_utils/island_logger.py +++ b/monkey/monkey_island/cc/server_utils/island_logger.py @@ -48,6 +48,6 @@ def setup_logging(data_dir_path, log_level): logger_configuration["handlers"]["file_handler"]["filename"] = os.path.join( data_dir_path, ISLAND_LOG_FILENAME ) - logger_configuration["root"]["level"] = log_level + logger_configuration["root"]["level"] = log_level.upper() logging.config.dictConfig(logger_configuration) diff --git a/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py b/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py index c301f3d5b..9f4e59af8 100644 --- a/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py +++ b/monkey/tests/monkey_island/cc/server_utils/test_island_logger.py @@ -64,3 +64,17 @@ def test_setup_logging_console_log_level_info(capsys, tmpdir): captured = capsys.readouterr() assert TEST_STRING not in captured.out + + +def test_setup_logging_console_log_level_lower_case(capsys, tmpdir): + DATA_DIR = tmpdir + LOG_LEVEL = "debug" + TEST_STRING = "Hello, Monkey! (Console; Log level: debug)" + + setup_logging(DATA_DIR, LOG_LEVEL) + + logger = logging.getLogger("TestLogger") + logger.debug(TEST_STRING) + + captured = capsys.readouterr() + assert TEST_STRING in captured.out From b13839d7babf0ab6fd09683dc93c6f9ac626b91a Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 11 May 2021 14:01:43 -0400 Subject: [PATCH 20/20] island: Add debug log level to server_config.json.develop --- monkey/monkey_island/cc/server_config.json.develop | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/monkey/monkey_island/cc/server_config.json.develop b/monkey/monkey_island/cc/server_config.json.develop index ecc4c1708..33fb33487 100644 --- a/monkey/monkey_island/cc/server_config.json.develop +++ b/monkey/monkey_island/cc/server_config.json.develop @@ -1,4 +1,5 @@ { "server_config": "password", - "deployment": "develop" + "deployment": "develop", + "log_level": "DEBUG" }