Merge branch 'untangle-logger-config' into develop

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

appimage/island_logger_config.json

@@ -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"
-    ]
-  }
-}

monkey/monkey_island.py

@@ -5,8 +5,10 @@ from monkey_island.cc.arg_parser import parse_cli_args
 gevent_monkey.patch_all()
 
 import json  # noqa: E402
+import os  # noqa: E402
 
-from monkey_island.cc.server_utils.island_logger import json_setup_logging  # noqa: E402
+from monkey_island import config_loader  # noqa: E402
+from monkey_island.cc.server_utils.island_logger import setup_logging  # noqa: E402
 
 if "__main__" == __name__:
     island_args = parse_cli_args()
@@ -14,9 +16,18 @@ 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)
+
+        config = config_loader.load_server_config_from_file(server_config_path)
+
+        setup_logging(config["data_dir"], config["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

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)

monkey/monkey_island/cc/island_logger_default_config.json

@@ -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"
-    ]
-  }
-}

monkey/monkey_island/cc/server_config.json.develop

@@ -1,4 +1,5 @@
 {
   "server_config": "password",
-  "deployment": "develop"
+  "deployment": "develop",
+  "log_level": "DEBUG"
 }

monkey/monkey_island/cc/server_utils/consts.py

@@ -11,8 +11,4 @@ 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_DATA_DIR = os.path.join(MONKEY_ISLAND_ABS_PATH, "cc")

monkey/monkey_island/cc/server_utils/island_logger.py

@@ -1,43 +1,53 @@
-import json
 import logging.config
 import os
-from typing import Dict
+from copy import deepcopy
 
-from monkey_island.cc.server_utils.consts import DEFAULT_LOGGER_CONFIG_PATH
+ISLAND_LOG_FILENAME = "monkey_island.log"
 
-__author__ = "Maor.Rayzin"
+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_handler": {
+            "class": "logging.StreamHandler",
+            "formatter": "simple",
+            "stream": "ext://sys.stdout",
+        },
+        "file_handler": {
+            "class": "logging.handlers.RotatingFileHandler",
+            "formatter": "simple",
+            "filename": None,  # set in setup_logging()
+            "maxBytes": 10485760,
+            "backupCount": 20,
+            "encoding": "utf8",
+        },
+    },
+    "root": {
+        "level": None,  # set in setup_logging()
+        "handlers": ["console_handler", "file_handler"],
+    },
+}
 
 
-def json_setup_logging(
-    default_path=DEFAULT_LOGGER_CONFIG_PATH,
-    default_level=logging.INFO,
-    env_key="LOG_CFG",
-):
+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
+    logger_configuration = deepcopy(LOGGER_CONFIG_DICT)
 
-    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["handlers"]["file_handler"]["filename"] = os.path.join(
+        data_dir_path, ISLAND_LOG_FILENAME
+    )
+    logger_configuration["root"]["level"] = log_level.upper()
 
-
-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"])
+    logging.config.dictConfig(logger_configuration)

monkey/monkey_island/config_loader.py

@@ -0,0 +1,25 @@
+import json
+import os
+
+from monkey_island.cc.server_utils.consts import DEFAULT_DATA_DIR
+
+DEFAULT_LOG_LEVEL = "INFO"
+
+
+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)
+        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.get("data_dir", DEFAULT_DATA_DIR)))
+    )
+
+    config.setdefault("log_level", DEFAULT_LOG_LEVEL)
+
+    return config

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

monkey/tests/monkey_island/cc/server_utils/test_island_logger.py

@@ -1,32 +1,80 @@
 import logging
 import os
 
-import pytest
-
-from monkey_island.cc.server_utils.island_logger import json_setup_logging
+from monkey_island.cc.server_utils.island_logger import ISLAND_LOG_FILENAME, setup_logging
 
 
-@pytest.fixture()
-def test_logger_config_path(resources_dir):
-    return os.path.join(resources_dir, "logger_config.json")
+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! (File; Log level: debug)"
 
-
-# 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, test_logger_config_path):
-    INFO_LOG = os.path.join(tmpdir, "info.log")
-    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)
+    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_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! (File; 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
+
+
+def test_setup_logging_console_log_level_debug(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
+
+
+def test_setup_logging_console_log_level_info(capsys, tmpdir):
+    DATA_DIR = tmpdir
+    LOG_LEVEL = "INFO"
+    TEST_STRING = "Hello, Monkey! (Console; 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
+
+
+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

monkey/tests/monkey_island/test_config_loader.py

@@ -0,0 +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_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

monkey/tests/resources/server_configs/test_server_config.json

@@ -0,0 +1,4 @@
+{
+    "data_dir": "~/.monkey_island",
+    "log_level": "NOTICE"
+}