From e95df875beba116d8082b5498fc124d2d576d87b Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Tue, 30 Nov 2021 10:27:09 +0200 Subject: [PATCH] Island: fix a bug in server's config options extraction that caused unspecified properties to get overridden by defaults --- monkey/monkey_island/cc/setup/config_setup.py | 5 ++- .../cc/setup/island_config_options.py | 4 +-- .../cc/setup/test_server_setup.py | 35 +++++++++++++------ 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/monkey/monkey_island/cc/setup/config_setup.py b/monkey/monkey_island/cc/setup/config_setup.py index 3dca8e3dc..c38ee791b 100644 --- a/monkey/monkey_island/cc/setup/config_setup.py +++ b/monkey/monkey_island/cc/setup/config_setup.py @@ -37,8 +37,7 @@ def _update_config_from_file(config: IslandConfigOptions, config_path: Path): logger.info(f"Server config not found in path {config_path}") -def _load_server_config_from_file(server_config_path) -> IslandConfigOptions: +def _load_server_config_from_file(server_config_path) -> dict: with open(server_config_path, "r") as f: config_content = f.read() - config = json.loads(config_content) - return IslandConfigOptions(config) + return json.loads(config_content) diff --git a/monkey/monkey_island/cc/setup/island_config_options.py b/monkey/monkey_island/cc/setup/island_config_options.py index a9cf66785..71e9ba27f 100644 --- a/monkey/monkey_island/cc/setup/island_config_options.py +++ b/monkey/monkey_island/cc/setup/island_config_options.py @@ -34,5 +34,5 @@ class IslandConfigOptions: ) ) - def update(self, target: IslandConfigOptions): - self.__dict__.update(target.__dict__) + def update(self, target: dict): + self.__dict__.update(target) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_server_setup.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_server_setup.py index e564c7c81..7517aa665 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_server_setup.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_server_setup.py @@ -43,10 +43,10 @@ def mock_deployment_config_path(monkeypatch, deployment_server_config_path): @pytest.fixture(autouse=True) -def mock_user_config_path(monkeypatch, deployment_server_config_path): +def mock_user_config_path(monkeypatch, user_default_server_config_path): monkeypatch.setattr( "monkey_island.cc.setup.config_setup.USER_CONFIG_PATH", - deployment_server_config_path, + user_default_server_config_path, ) @@ -59,8 +59,8 @@ def test_extract_config_defaults(): def test_deployment_config_overrides_defaults(deployment_server_config_path): - expected = IslandConfigOptions({"key_path": "/key_path_2"}) - create_server_config(dumps({"key_path": "/key_path_2"}), deployment_server_config_path) + expected = IslandConfigOptions({"log_level": "/log_level_2"}) + create_server_config(dumps({"log_level": "/log_level_2"}), deployment_server_config_path) assert ( expected.__dict__ == _extract_config(IslandCmdArgs(setup_only=False, server_config_path=None)).__dict__ @@ -70,9 +70,9 @@ def test_deployment_config_overrides_defaults(deployment_server_config_path): def test_user_config_overrides_deployment( deployment_server_config_path, cmd_server_config_path, user_default_server_config_path ): - expected = IslandConfigOptions({"key_path": "/key_path_3"}) - create_server_config(dumps({"key_path": "/key_path_2"}), deployment_server_config_path) - create_server_config(dumps({"key_path": "/key_path_3"}), user_default_server_config_path) + expected = IslandConfigOptions({"log_level": "/log_level_3"}) + create_server_config(dumps({"log_level": "/log_level_2"}), deployment_server_config_path) + create_server_config(dumps({"log_level": "/log_level_3"}), user_default_server_config_path) extracted_config = _extract_config( IslandCmdArgs(setup_only=False, server_config_path=cmd_server_config_path) ) @@ -82,10 +82,23 @@ def test_user_config_overrides_deployment( def test_cmd_config_overrides_everything( deployment_server_config_path, cmd_server_config_path, user_default_server_config_path ): - expected = IslandConfigOptions({"key_path": "/key_path_4"}) - create_server_config(dumps({"key_path": "/key_path_2"}), deployment_server_config_path) - create_server_config(dumps({"key_path": "/key_path_3"}), user_default_server_config_path) - create_server_config(dumps({"key_path": "/key_path_4"}), cmd_server_config_path) + expected = IslandConfigOptions({"log_level": "/log_level_4"}) + create_server_config(dumps({"log_level": "/log_level_2"}), deployment_server_config_path) + create_server_config(dumps({"log_level": "/log_level_3"}), user_default_server_config_path) + create_server_config(dumps({"log_level": "/log_level_4"}), cmd_server_config_path) + extracted_config = _extract_config( + IslandCmdArgs(setup_only=False, server_config_path=cmd_server_config_path) + ) + assert expected.__dict__ == extracted_config.__dict__ + + +def test_not_overriding_unspecified_values( + deployment_server_config_path, cmd_server_config_path, user_default_server_config_path +): + expected = IslandConfigOptions({"log_level": "/log_level_4", "data_dir": "/data_dir1"}) + create_server_config(dumps({"data_dir": "/data_dir1"}), deployment_server_config_path) + create_server_config(dumps({"log_level": "/log_level_3"}), user_default_server_config_path) + create_server_config(dumps({"log_level": "/log_level_4"}), cmd_server_config_path) extracted_config = _extract_config( IslandCmdArgs(setup_only=False, server_config_path=cmd_server_config_path) )