From 4c80085d8c4f4c14380190e8648188969ff8cdf4 Mon Sep 17 00:00:00 2001 From: Francis Charette Migneault Date: Fri, 6 Oct 2023 00:22:34 -0400 Subject: [PATCH] add notification email tests + fix encrypt/decrypt email procedure (fixes #568) --- CHANGES.rst | 7 +++- config/weaver.ini.example | 1 + tests/test_notify.py | 76 +++++++++++++++++++++++++++++++++------ weaver/datatype.py | 13 ++++--- weaver/notify.py | 27 ++++++++------ 5 files changed, 94 insertions(+), 30 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index cdcd6a23e..248cb53ad 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -13,10 +13,15 @@ Changes Changes: -------- - Add utility methods for `Job` to easily retrieve its various URLs. +- Add ``weaver.wps_email_notify_timeout`` setting (default 10s) to avoid SMTP server deadlock on failing connection. +- Modify the ``encrypt_email`` function to use an alternate strategy allowing ``decrypt_email`` on `Job` completed. Fixes: ------ -- No change. +- Fix `Job` submitted email encryption not reversible to retrieve the original notification email on completion + (fixes `#568 `_). +- Fix example Mako Template for email notification using an unavailable property ``${logs}``. + Instead, the new utility methods ``job.[...]_url`` should be used to retrieve relevant locations. .. _changes_4.32.0: diff --git a/config/weaver.ini.example b/config/weaver.ini.example index 65c7a099f..e2cb1ecf0 100644 --- a/config/weaver.ini.example +++ b/config/weaver.ini.example @@ -148,6 +148,7 @@ weaver.wps_email_encrypt_rounds = 100000 weaver.wps_email_notify_smtp_host = weaver.wps_email_notify_from_addr = example@email.com weaver.wps_email_notify_password = 123456 +weaver.wps_email_notify_timeout = 10 weaver.wps_email_notify_port = 25 weaver.wps_email_notify_ssl = true weaver.wps_email_notify_template_dir = diff --git a/tests/test_notify.py b/tests/test_notify.py index 8ac48f207..c1062395f 100644 --- a/tests/test_notify.py +++ b/tests/test_notify.py @@ -45,12 +45,14 @@ def test_encrypt_decrypt_email_raise(email_func): def test_notify_job_complete(): + test_url = "https://test-weaver.example.com" settings = { - "weaver.url": "test-weaver.example.com", + "weaver.url": test_url, "weaver.wps_email_notify_smtp_host": "xyz.test.com", "weaver.wps_email_notify_from_addr": "test-weaver@email.com", "weaver.wps_email_notify_password": "super-secret", "weaver.wps_email_notify_port": 12345, + "weaver.wps_email_notify_timeout": 1, # quick fail if invalid } notify_email = "test-user@email.com" test_job = Job( @@ -58,33 +60,85 @@ def test_notify_job_complete(): process="test-process", settings=settings, ) + test_job_err_url = f"{test_url}/processes/{test_job.process}/jobs/{test_job.id}/exceptions" + test_job_out_url = f"{test_url}/processes/{test_job.process}/jobs/{test_job.id}/results" + test_job_log_url = f"{test_url}/processes/{test_job.process}/jobs/{test_job.id}/logs" with mock.patch("smtplib.SMTP_SSL", autospec=smtplib.SMTP_SSL) as mock_smtp: - mock_smtp.sendmail = mock.MagicMock(return_value=None) # sending worked + mock_smtp.return_value.sendmail.return_value = None # sending worked test_job.status = Status.SUCCEEDED - test_job.progress = 100 notify_job_complete(test_job, notify_email, settings) mock_smtp.assert_called_with("xyz.test.com", 12345) - - # test_job.status = Status.FAILED - # test_job.progress = 42 - # notify_job_complete(test_job, notify_email, settings) + assert mock_smtp.return_value.sendmail.call_args[0][0] == "test-weaver@email.com" + assert mock_smtp.return_value.sendmail.call_args[0][1] == notify_email + message_encoded = mock_smtp.return_value.sendmail.call_args[0][2] + assert message_encoded + message = message_encoded.decode("utf8") + assert "From: Weaver" in message + assert f"To: {notify_email}" in message + assert f"Subject: Job {test_job.process} Succeeded" + assert test_job_out_url in message + assert test_job_log_url in message + assert test_job_err_url not in message + + test_job.status = Status.FAILED + notify_job_complete(test_job, notify_email, settings) + assert mock_smtp.return_value.sendmail.call_args[0][0] == "test-weaver@email.com" + assert mock_smtp.return_value.sendmail.call_args[0][1] == notify_email + message_encoded = mock_smtp.return_value.sendmail.call_args[0][2] + assert message_encoded + message = message_encoded.decode("utf8") + assert "From: Weaver" in message + assert f"To: {notify_email}" in message + assert f"Subject: Job {test_job.process} Failed" + assert test_job_out_url not in message + assert test_job_log_url in message + assert test_job_err_url in message def test_notify_job_complete_custom_template(): with tempfile.NamedTemporaryFile(mode="w", encoding="utf-8", suffix=".mako") as email_template_file: + email_template_file.writelines([ + "From: Weaver\n", + "To: ${to}\n", + "Subject: Job ${job.process} ${job.status}\n", + "\n", # end of email header, content below + "Job: ${job.status_url(settings)}\n", + ]) + email_template_file.flush() + email_template_file.seek(0) + mako_dir, mako_name = os.path.split(email_template_file.name) + test_url = "https://test-weaver.example.com" settings = { - "weaver.url": "test-weaver.example.com", + "weaver.url": test_url, "weaver.wps_email_notify_smtp_host": "xyz.test.com", "weaver.wps_email_notify_from_addr": "test-weaver@email.com", "weaver.wps_email_notify_password": "super-secret", "weaver.wps_email_notify_port": 12345, + "weaver.wps_email_notify_timeout": 1, # quick fail if invalid "weaver.wps_email_notify_template_dir": mako_dir, "weaver.wps_email_notify_template_default": mako_name, } - with mock.patch("smtplib.SMTP_SSL", autospec=smtplib.SMTP_SSL) as mock_smtp: - mock_smtp.sendmail = mock.MagicMock(return_value=None) # sending worked + notify_email = "test-user@email.com" + test_job = Job( + task_id=uuid.uuid4(), + process="test-process", + status=Status.SUCCEEDED, + settings=settings, + ) - mock_smtp.ass + with mock.patch("smtplib.SMTP_SSL", autospec=smtplib.SMTP_SSL) as mock_smtp: + mock_smtp.return_value.sendmail.return_value = None # sending worked + notify_job_complete(test_job, notify_email, settings) + + message_encoded = mock_smtp.return_value.sendmail.call_args[0][2] + message = message_encoded.decode("utf8") + assert message == "\n".join([ + "From: Weaver", + f"To: {notify_email}", + f"Subject: Job {test_job.process} {Status.SUCCEEDED}", + "", + f"Job: {test_url}/processes/{test_job.process}/jobs/{test_job.id}", + ]) diff --git a/weaver/datatype.py b/weaver/datatype.py index 941b788b8..85a32aa89 100644 --- a/weaver/datatype.py +++ b/weaver/datatype.py @@ -1236,8 +1236,7 @@ def job_url(self, container=None, extra_path=None): # type: (Optional[AnySettingsContainer], Optional[str]) -> str settings = get_settings(container) base_url = get_wps_restapi_base_url(settings) - job_path = f"{extra_path}" if extra_path else "" - return self._job_url(base_url) + job_path + return self._job_url(base_url) + (extra_path or "") def status_url(self, container=None): # type: (Optional[AnySettingsContainer]) -> str @@ -1245,23 +1244,23 @@ def status_url(self, container=None): def logs_url(self, container=None): # type: (Optional[AnySettingsContainer]) -> str - return self.job_url(container=container, extra_path="logs") + return self.job_url(container=container, extra_path="/logs") def exceptions_url(self, container=None): # type: (Optional[AnySettingsContainer]) -> str - return self.job_url(container=container, extra_path="exceptions") + return self.job_url(container=container, extra_path="/exceptions") def inputs_url(self, container=None): # type: (Optional[AnySettingsContainer]) -> str - return self.job_url(container=container, extra_path="inputs") + return self.job_url(container=container, extra_path="/inputs") def outputs_url(self, container=None): # type: (Optional[AnySettingsContainer]) -> str - return self.job_url(container=container, extra_path="outputs") + return self.job_url(container=container, extra_path="/outputs") def results_url(self, container=None): # type: (Optional[AnySettingsContainer]) -> str - return self.job_url(container=container, extra_path="results") + return self.job_url(container=container, extra_path="/results") def links(self, container=None, self_link=None): # type: (Optional[AnySettingsContainer], Optional[str]) -> List[Link] diff --git a/weaver/notify.py b/weaver/notify.py index 516a3eb18..5c0601fc0 100644 --- a/weaver/notify.py +++ b/weaver/notify.py @@ -31,14 +31,17 @@ job: weaver.datatype.Job object settings: application settings - And every variable returned by the `weaver.datatype.Job.json` method: - status: succeeded, failed - logs: url to the logs - jobID: example "617f23d3-f474-47f9-a8ec-55da9dd6ac71" - result: url to the outputs - duration: example "0:01:02" - message: example "Job succeeded." - percentCompleted: example 100 + And every variable returned by the `weaver.datatype.Job.json` method. + Below is a non-exhaustive list of example parameters from this method. + Refer to the method for complete listing. + + status: succeeded, failed + logs: url to the logs + jobID: example "617f23d3-f474-47f9-a8ec-55da9dd6ac71" + result: url to the outputs + duration: example "0:01:02" + message: example "Job succeeded." + percentCompleted: example 100 From: Weaver To: ${to} @@ -76,11 +79,12 @@ def notify_job_complete(job, to_email_recipient, container): smtp_host = settings.get("weaver.wps_email_notify_smtp_host") from_addr = settings.get("weaver.wps_email_notify_from_addr") password = settings.get("weaver.wps_email_notify_password") + timeout = int(settings.get("weaver.wps_email_notify_timeout") or 10) port = settings.get("weaver.wps_email_notify_port") ssl = asbool(settings.get("weaver.wps_email_notify_ssl", True)) # an example template is located in # weaver/wps_restapi/templates/notification_email_example.mako - template_dir = settings.get("weaver.wps_email_notify_template_dir") + template_dir = settings.get("weaver.wps_email_notify_template_dir") or "" if not smtp_host or not port: raise ValueError("The email server configuration is missing.") @@ -107,9 +111,9 @@ def notify_job_complete(job, to_email_recipient, container): message = f"{contents}".strip("\n") if ssl: - server = smtplib.SMTP_SSL(smtp_host, port) + server = smtplib.SMTP_SSL(smtp_host, port, timeout=timeout) else: - server = smtplib.SMTP(smtp_host, port) + server = smtplib.SMTP(smtp_host, port, timeout=timeout) server.ehlo() try: server.starttls() @@ -129,6 +133,7 @@ def notify_job_complete(job, to_email_recipient, container): raise IOError(f"Code: {code}, Message: {error_message}") +# https://stackoverflow.com/a/55147077 def get_crypto_key(settings, salt, rounds): # type: (SettingsType, bytes, int) -> bytes """