From 73ff81e180504578821bef66091188840233bc86 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 4 May 2020 17:19:12 +0200 Subject: [PATCH 1/6] append string to Message text for disabled messages --- .../python/opengrok_tools/utils/mirror.py | 24 +++++++++++++++---- opengrok-tools/src/test/python/test_mirror.py | 4 ++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py b/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py index 99fd365e9a2..4e1531191e1 100644 --- a/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py +++ b/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py @@ -304,14 +304,23 @@ def run_command(cmd, project_name): cmd.getoutputstr())) -def handle_disabled_project(config, project_name): +def handle_disabled_project(config, project_name, disabled_msg): disabled_command = config.get(DISABLED_CMD_PROPERTY) if disabled_command: logger = logging.getLogger(__name__) logger.debug("Calling disabled command: {}".format(disabled_command)) command_args = disabled_command.get(COMMAND_PROPERTY) - if is_web_uri(command_args[0]): + uri = command_args[0] + if is_web_uri(uri): + # Is this perhaps OpenGrok API call to supply a Message ? + # If so and there was a string supplied, append it + # to the message text. + text = command_args[2].get("text") + if uri.find("/api/v1/") > 0 and disabled_msg and type(disabled_msg) is str and text: + logger.debug("Appending text to message: {}".format(disabled_msg)) + command_args[2]["text"] = text + ": " + disabled_msg + r = call_rest_api(disabled_command, PROJECT_SUBST, project_name) try: r.raise_for_status() @@ -320,11 +329,15 @@ def handle_disabled_project(config, project_name): "project '{}': {}". format(project_name, r)) else: + args = [project_name] + if disabled_msg and type(disabled_msg) is str: + args.append(disabled_command) + cmd = Command(command_args, env_vars=disabled_command.get("env"), resource_limits=disabled_command.get("limits"), args_subst={PROJECT_SUBST: project_name}, - args_append=[project_name], excl_subst=True) + args_append=args, excl_subst=True) run_command(cmd, project_name) @@ -362,8 +375,9 @@ def mirror_project(config, project_name, check_changes, uri, # We want this to be logged to the log file (if any). if project_config: - if project_config.get(DISABLED_PROPERTY): - handle_disabled_project(config, project_name) + disabled_value = project_config.get(DISABLED_PROPERTY) + if disabled_value: + handle_disabled_project(config, project_name, disabled_value) logger.info("Project '{}' disabled, exiting". format(project_name)) return CONTINUE_EXITVAL diff --git a/opengrok-tools/src/test/python/test_mirror.py b/opengrok-tools/src/test/python/test_mirror.py index 8e562833b30..6edc75870b6 100644 --- a/opengrok-tools/src/test/python/test_mirror.py +++ b/opengrok-tools/src/test/python/test_mirror.py @@ -181,6 +181,10 @@ def test_empty_project_config(): def test_disabled_command_api(): + """ + Test that mirror_project() calls call_rest_api() if API + call is specified in the configuration for disabled project. + """ with patch(opengrok_tools.utils.mirror.call_rest_api, lambda a, b, c: mock(spec=requests.Response)): project_name = "foo" From db7aa5e7fdfb242cac8d68511cdab110208052be Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 4 May 2020 18:12:41 +0200 Subject: [PATCH 2/6] reorder evaluation --- opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py b/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py index 4e1531191e1..277d6910c92 100644 --- a/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py +++ b/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py @@ -317,7 +317,7 @@ def handle_disabled_project(config, project_name, disabled_msg): # If so and there was a string supplied, append it # to the message text. text = command_args[2].get("text") - if uri.find("/api/v1/") > 0 and disabled_msg and type(disabled_msg) is str and text: + if text and uri.find("/api/v1/") > 0 and type(disabled_msg) is str: logger.debug("Appending text to message: {}".format(disabled_msg)) command_args[2]["text"] = text + ": " + disabled_msg From 51aa02b6eab716f44ffe95d0e9ab5bc1813f349f Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 4 May 2020 18:16:45 +0200 Subject: [PATCH 3/6] be more careful with message payload --- .../src/main/python/opengrok_tools/utils/mirror.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py b/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py index 277d6910c92..3a8d69759d1 100644 --- a/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py +++ b/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py @@ -316,7 +316,10 @@ def handle_disabled_project(config, project_name, disabled_msg): # Is this perhaps OpenGrok API call to supply a Message ? # If so and there was a string supplied, append it # to the message text. - text = command_args[2].get("text") + data = command_args[2] + text = None + if type(data) is dict: + text = data.get("text") if text and uri.find("/api/v1/") > 0 and type(disabled_msg) is str: logger.debug("Appending text to message: {}".format(disabled_msg)) command_args[2]["text"] = text + ": " + disabled_msg From 16092970b717adb5601f2f77f4a0e969f97cabe1 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 5 May 2020 09:49:00 +0200 Subject: [PATCH 4/6] add unit test --- opengrok-tools/requirements.txt | 2 +- opengrok-tools/src/test/python/test_mirror.py | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/opengrok-tools/requirements.txt b/opengrok-tools/requirements.txt index 6b79f70d388..0aa6bbc891b 100644 --- a/opengrok-tools/requirements.txt +++ b/opengrok-tools/requirements.txt @@ -11,4 +11,4 @@ requests==2.20.0 Resource==0.2.1 six==1.11.0 urllib3==1.23 -GitPython==3.0.2 \ No newline at end of file +GitPython==3.0.6 diff --git a/opengrok-tools/src/test/python/test_mirror.py b/opengrok-tools/src/test/python/test_mirror.py index 6edc75870b6..5fab5d34876 100644 --- a/opengrok-tools/src/test/python/test_mirror.py +++ b/opengrok-tools/src/test/python/test_mirror.py @@ -201,6 +201,42 @@ def test_disabled_command_api(): PROJECT_SUBST, project_name) +def test_disabled_command_api_text_append(monkeypatch): + """ + Test that message text is appended if DISABLED_PROPERTY is a string. + """ + + text_to_append = "foo bar" + + def mock_call_rest_api(command, b, c): + disabled_command = config.get(DISABLED_CMD_PROPERTY) + assert disabled_command + command_args = disabled_command.get(COMMAND_PROPERTY) + assert command_args + data = command_args[2] + assert data + text = data.get("text") + assert text + assert text.find(text_to_append) + + return mock(spec=requests.Response) + + with monkeypatch.context() as m: + m.setattr("opengrok_tools.utils.mirror.call_rest_api", + mock_call_rest_api) + + project_name = "foo" + data = {'messageLevel': 'info', 'duration': 'PT5M', 'tags': ['%PROJECT%'], + 'text': 'disabled project'} + config = {DISABLED_CMD_PROPERTY: + {COMMAND_PROPERTY: + ["http://localhost:8080/source/api/v1/foo", + "POST", data]}, + PROJECTS_PROPERTY: {project_name: {DISABLED_PROPERTY: text_to_append}}} + + mirror_project(config, project_name, False, None, None) + + def test_disabled_command_run(): """ Make sure that mirror_project() results in calling run_command(). From 004ba9c2e92c59521a0fff8d4809aad8b44f601c Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 5 May 2020 10:17:12 +0200 Subject: [PATCH 5/6] do not overload disabled property --- .../src/main/python/opengrok_tools/utils/mirror.py | 10 ++++++---- opengrok-tools/src/test/python/test_mirror.py | 6 ++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py b/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py index 3a8d69759d1..f2efa789a09 100644 --- a/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py +++ b/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py @@ -54,6 +54,7 @@ PROXY_PROPERTY = 'proxy' COMMANDS_PROPERTY = 'commands' DISABLED_PROPERTY = 'disabled' +DISABLED_REASON_PROPERTY = 'disabled-reason' HOOKDIR_PROPERTY = 'hookdir' HOOKS_PROPERTY = 'hooks' LOGDIR_PROPERTY = 'logdir' @@ -378,9 +379,9 @@ def mirror_project(config, project_name, check_changes, uri, # We want this to be logged to the log file (if any). if project_config: - disabled_value = project_config.get(DISABLED_PROPERTY) - if disabled_value: - handle_disabled_project(config, project_name, disabled_value) + if project_config.get(DISABLED_PROPERTY): + handle_disabled_project(config, project_name, + project_config.get(DISABLED_REASON_PROPERTY)) logger.info("Project '{}' disabled, exiting". format(project_name)) return CONTINUE_EXITVAL @@ -446,7 +447,8 @@ def check_project_configuration(multiple_project_config, hookdir=False, # Quick sanity check. known_project_tunables = [DISABLED_PROPERTY, CMD_TIMEOUT_PROPERTY, HOOK_TIMEOUT_PROPERTY, PROXY_PROPERTY, - IGNORED_REPOS_PROPERTY, HOOKS_PROPERTY] + IGNORED_REPOS_PROPERTY, HOOKS_PROPERTY, + DISABLED_REASON_PROPERTY] if not multiple_project_config: return True diff --git a/opengrok-tools/src/test/python/test_mirror.py b/opengrok-tools/src/test/python/test_mirror.py index 5fab5d34876..9f3a7378472 100644 --- a/opengrok-tools/src/test/python/test_mirror.py +++ b/opengrok-tools/src/test/python/test_mirror.py @@ -39,7 +39,7 @@ check_configuration, mirror_project, run_command, get_repos_for_project, \ HOOKS_PROPERTY, PROXY_PROPERTY, IGNORED_REPOS_PROPERTY, \ PROJECTS_PROPERTY, DISABLED_CMD_PROPERTY, DISABLED_PROPERTY, \ - CMD_TIMEOUT_PROPERTY, HOOK_TIMEOUT_PROPERTY + CMD_TIMEOUT_PROPERTY, HOOK_TIMEOUT_PROPERTY, DISABLED_REASON_PROPERTY import opengrok_tools.mirror from opengrok_tools.utils.exitvals import ( CONTINUE_EXITVAL, FAILURE_EXITVAL @@ -232,7 +232,9 @@ def mock_call_rest_api(command, b, c): {COMMAND_PROPERTY: ["http://localhost:8080/source/api/v1/foo", "POST", data]}, - PROJECTS_PROPERTY: {project_name: {DISABLED_PROPERTY: text_to_append}}} + PROJECTS_PROPERTY: {project_name: + {DISABLED_REASON_PROPERTY: text_to_append, + DISABLED_PROPERTY: True}}} mirror_project(config, project_name, False, None, None) From 5fd773adfc07e1d0dfd7e2119b26532a7d6688a4 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 5 May 2020 11:25:16 +0200 Subject: [PATCH 6/6] fix verify --- .../src/main/python/opengrok_tools/utils/mirror.py | 6 ++++-- opengrok-tools/src/test/python/test_mirror.py | 14 ++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py b/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py index f2efa789a09..9c03bcddb01 100644 --- a/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py +++ b/opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py @@ -322,7 +322,8 @@ def handle_disabled_project(config, project_name, disabled_msg): if type(data) is dict: text = data.get("text") if text and uri.find("/api/v1/") > 0 and type(disabled_msg) is str: - logger.debug("Appending text to message: {}".format(disabled_msg)) + logger.debug("Appending text to message: {}". + format(disabled_msg)) command_args[2]["text"] = text + ": " + disabled_msg r = call_rest_api(disabled_command, PROJECT_SUBST, project_name) @@ -381,7 +382,8 @@ def mirror_project(config, project_name, check_changes, uri, if project_config: if project_config.get(DISABLED_PROPERTY): handle_disabled_project(config, project_name, - project_config.get(DISABLED_REASON_PROPERTY)) + project_config. + get(DISABLED_REASON_PROPERTY)) logger.info("Project '{}' disabled, exiting". format(project_name)) return CONTINUE_EXITVAL diff --git a/opengrok-tools/src/test/python/test_mirror.py b/opengrok-tools/src/test/python/test_mirror.py index 9f3a7378472..cd219c0060c 100644 --- a/opengrok-tools/src/test/python/test_mirror.py +++ b/opengrok-tools/src/test/python/test_mirror.py @@ -226,15 +226,17 @@ def mock_call_rest_api(command, b, c): mock_call_rest_api) project_name = "foo" - data = {'messageLevel': 'info', 'duration': 'PT5M', 'tags': ['%PROJECT%'], + data = {'messageLevel': 'info', 'duration': 'PT5M', + 'tags': ['%PROJECT%'], 'text': 'disabled project'} config = {DISABLED_CMD_PROPERTY: - {COMMAND_PROPERTY: - ["http://localhost:8080/source/api/v1/foo", - "POST", data]}, + {COMMAND_PROPERTY: + ["http://localhost:8080/source/api/v1/foo", + "POST", data]}, PROJECTS_PROPERTY: {project_name: - {DISABLED_REASON_PROPERTY: text_to_append, - DISABLED_PROPERTY: True}}} + {DISABLED_REASON_PROPERTY: + text_to_append, + DISABLED_PROPERTY: True}}} mirror_project(config, project_name, False, None, None)