From fa6d2ca91b35d556273876894d1eb5e9d1f6d037 Mon Sep 17 00:00:00 2001 From: juandiego-bmu Date: Fri, 3 Apr 2026 16:50:28 +0200 Subject: [PATCH 1/2] Fix mutable default argument in merge_logs_to_list Replace log_list=[] with log_list=None and initialize inside the function body. The mutable default was shared across all calls, causing log entries from previous invocations to leak into subsequent results and growing without bound. Fixes #1464 --- nettacker/core/utils/common.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nettacker/core/utils/common.py b/nettacker/core/utils/common.py index f88259daf..91f5f9c40 100644 --- a/nettacker/core/utils/common.py +++ b/nettacker/core/utils/common.py @@ -31,7 +31,9 @@ def replace_dependent_response(log, response_dependent): return log -def merge_logs_to_list(result, log_list=[]): +def merge_logs_to_list(result, log_list=None): + if log_list is None: + log_list = [] if isinstance(result, dict): if "json_event" in list(result.keys()): if not isinstance(result["json_event"], dict): From 6957125812822f29e8c7433790221b5a0ae6fb18 Mon Sep 17 00:00:00 2001 From: juandiego-bmu Date: Sat, 4 Apr 2026 10:47:02 +0200 Subject: [PATCH 2/2] Add docstring and tests for merge_logs_to_list Address review feedback: add docstring to meet coverage threshold and add tests verifying correct behavior including no shared state between consecutive calls (the core bug this PR fixes). --- nettacker/core/utils/common.py | 10 +++++++++ tests/core/utils/test_common.py | 37 +++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/nettacker/core/utils/common.py b/nettacker/core/utils/common.py index 91f5f9c40..e38de205e 100644 --- a/nettacker/core/utils/common.py +++ b/nettacker/core/utils/common.py @@ -32,6 +32,16 @@ def replace_dependent_response(log, response_dependent): def merge_logs_to_list(result, log_list=None): + """Recursively extract all 'log' values from a nested dict into a flat deduplicated list. + + Args: + result: A dict (possibly nested) containing 'log' keys to extract. + log_list: Accumulator list for recursive calls. Defaults to a new empty list + on each top-level call to avoid mutable default argument pitfalls. + + Returns: + A deduplicated list of extracted log values. + """ if log_list is None: log_list = [] if isinstance(result, dict): diff --git a/tests/core/utils/test_common.py b/tests/core/utils/test_common.py index f57f5e596..eb382b72c 100644 --- a/tests/core/utils/test_common.py +++ b/tests/core/utils/test_common.py @@ -92,3 +92,40 @@ def test_select_maximum_cpu_core(cpu_count_mock): for level in ("low", "normal", "high", "maximum"): assert common_utils.select_maximum_cpu_core(level) == levels[level] assert common_utils.select_maximum_cpu_core("invalid") == 1 + + +def test_merge_logs_to_list_simple(): + result = {"log": "error occurred"} + assert common_utils.merge_logs_to_list(result) == ["error occurred"] + + +def test_merge_logs_to_list_nested(): + result = { + "log": "outer", + "nested": {"log": "inner"}, + } + logs = common_utils.merge_logs_to_list(result) + assert sorted(logs) == ["inner", "outer"] + + +def test_merge_logs_to_list_no_log_key(): + result = {"status": "ok", "data": {"value": 42}} + assert common_utils.merge_logs_to_list(result) == [] + + +def test_merge_logs_to_list_deduplicates(): + result = { + "log": "same", + "nested": {"log": "same"}, + } + assert common_utils.merge_logs_to_list(result) == ["same"] + + +def test_merge_logs_to_list_no_shared_state_between_calls(): + """Verify that consecutive calls without explicit log_list don't leak state.""" + result_a = {"log": "first"} + result_b = {"log": "second"} + logs_a = common_utils.merge_logs_to_list(result_a) + logs_b = common_utils.merge_logs_to_list(result_b) + assert logs_a == ["first"] + assert logs_b == ["second"]