diff --git a/providers/src/airflow/providers/smtp/CHANGELOG.rst b/providers/src/airflow/providers/smtp/CHANGELOG.rst index 038fe52acaa0a..e6d91b7c7abf4 100644 --- a/providers/src/airflow/providers/smtp/CHANGELOG.rst +++ b/providers/src/airflow/providers/smtp/CHANGELOG.rst @@ -27,6 +27,28 @@ Changelog --------- + +2.0.0 +..... + +Breaking changes +~~~~~~~~~~~~~~~~ +.. warning:: + The argument ``from_email`` is now an optional kwarg in ``SmtpNotifier``, and the argument ``to`` became the first + positional argument. + + Configuring the ``SmtpNotifier`` and ``SmtpHook`` default values via Airflow SMTP configurations is not supported + anymore. You can instead use the SMTP connection configuration to set the default values, where you can use: + + * the connection extra field ``ssl_context`` instead of the configuration ``smtp_provider.ssl_context`` or + ``email.ssl_context`` in the SMTP hook. + * the connection extra field ``from_email`` instead of the configuration ``smtp.smtp_mail_from`` in ``SmtpNotifier``. + * the connection extra field ``subject_template`` instead of the configuration ``smtp.templated_email_subject_path`` + in ``SmtpNotifier``. + * the connection extra field ``html_content_template`` instead of the configuration + ``smtp.templated_html_content_path`` in ``SmtpNotifier``. + + 1.9.0 ..... diff --git a/providers/src/airflow/providers/smtp/hooks/smtp.py b/providers/src/airflow/providers/smtp/hooks/smtp.py index 74707fad5f0a7..991dea54867c9 100644 --- a/providers/src/airflow/providers/smtp/hooks/smtp.py +++ b/providers/src/airflow/providers/smtp/hooks/smtp.py @@ -114,25 +114,15 @@ def _build_client(self) -> smtplib.SMTP_SSL | smtplib.SMTP: smtp_kwargs["timeout"] = self.timeout if self.use_ssl: - from airflow.configuration import conf - - extra_ssl_context = self.conn.extra_dejson.get("ssl_context", None) - if extra_ssl_context: - ssl_context_string = extra_ssl_context - else: - ssl_context_string = conf.get("smtp_provider", "SSL_CONTEXT", fallback=None) - if ssl_context_string is None: - ssl_context_string = conf.get("email", "SSL_CONTEXT", fallback=None) - if ssl_context_string is None: - ssl_context_string = "default" - if ssl_context_string == "default": + ssl_context_string = self.ssl_context + if ssl_context_string is None or ssl_context_string == "default": ssl_context = ssl.create_default_context() elif ssl_context_string == "none": ssl_context = None else: raise RuntimeError( - f"The email.ssl_context configuration variable must " - f"be set to 'default' or 'none' and is '{ssl_context_string}'." + f"The connection extra field `ssl_context` must " + f"be set to 'default' or 'none' but it is set to '{ssl_context_string}'." ) smtp_kwargs["context"] = ssl_context return SMTP(**smtp_kwargs) @@ -411,6 +401,10 @@ def subject_template(self) -> str | None: def html_content_template(self) -> str | None: return self.conn.extra_dejson.get("html_content_template") + @property + def ssl_context(self) -> str | None: + return self.conn.extra_dejson.get("ssl_context") + @staticmethod def _read_template(template_path: str) -> str: """ diff --git a/providers/src/airflow/providers/smtp/notifications/smtp.py b/providers/src/airflow/providers/smtp/notifications/smtp.py index 01ca19f80e7a2..85f71172d2334 100644 --- a/providers/src/airflow/providers/smtp/notifications/smtp.py +++ b/providers/src/airflow/providers/smtp/notifications/smtp.py @@ -22,7 +22,6 @@ from pathlib import Path from typing import Any -from airflow.configuration import conf from airflow.notifications.basenotifier import BaseNotifier from airflow.providers.smtp.hooks.smtp import SmtpHook @@ -67,10 +66,8 @@ class SmtpNotifier(BaseNotifier): def __init__( self, - # TODO: Move from_email to keyword parameter in next major release so that users do not - # need to specify from_email. No argument here will lead to defaults from conf being used. - from_email: str | None, to: str | Iterable[str], + from_email: str | None = None, subject: str | None = None, html_content: str | None = None, files: list[str] | None = None, @@ -85,7 +82,7 @@ def __init__( ): super().__init__() self.smtp_conn_id = smtp_conn_id - self.from_email = from_email or conf.get("smtp", "smtp_mail_from") + self.from_email = from_email self.to = to self.files = files self.cc = cc @@ -110,16 +107,20 @@ def hook(self) -> SmtpHook: def notify(self, context): """Send a email via smtp server.""" fields_to_re_render = [] + if self.from_email is None: + if self.hook.from_email is not None: + self.from_email = self.hook.from_email + else: + raise ValueError("You should provide `from_email` or define it in the connection") + fields_to_re_render.append("from_email") if self.subject is None: smtp_default_templated_subject_path: str if self.hook.subject_template: smtp_default_templated_subject_path = self.hook.subject_template else: - smtp_default_templated_subject_path = conf.get( - "smtp", - "templated_email_subject_path", - fallback=(Path(__file__).parent / "templates" / "email_subject.jinja2").as_posix(), - ) + smtp_default_templated_subject_path = ( + Path(__file__).parent / "templates" / "email_subject.jinja2" + ).as_posix() self.subject = self._read_template(smtp_default_templated_subject_path) fields_to_re_render.append("subject") if self.html_content is None: @@ -127,11 +128,9 @@ def notify(self, context): if self.hook.html_content_template: smtp_default_templated_html_content_path = self.hook.html_content_template else: - smtp_default_templated_html_content_path = conf.get( - "smtp", - "templated_html_content_path", - fallback=(Path(__file__).parent / "templates" / "email.html").as_posix(), - ) + smtp_default_templated_html_content_path = ( + Path(__file__).parent / "templates" / "email.html" + ).as_posix() self.html_content = self._read_template(smtp_default_templated_html_content_path) fields_to_re_render.append("html_content") if fields_to_re_render: diff --git a/providers/src/airflow/providers/smtp/provider.yaml b/providers/src/airflow/providers/smtp/provider.yaml index c25064ae74c57..9face29ef6318 100644 --- a/providers/src/airflow/providers/smtp/provider.yaml +++ b/providers/src/airflow/providers/smtp/provider.yaml @@ -69,43 +69,3 @@ connection-types: notifications: - airflow.providers.smtp.notifications.smtp.SmtpNotifier - -config: - smtp_provider: - description: "Options for SMTP provider." - options: - ssl_context: - description: | - ssl context to use when using SMTP and IMAP SSL connections. By default, the context is "default" - which sets it to ``ssl.create_default_context()`` which provides the right balance between - compatibility and security, it however requires that certificates in your operating system are - updated and that SMTP/IMAP servers of yours have valid certificates that have corresponding public - keys installed on your machines. You can switch it to "none" if you want to disable checking - of the certificates, but it is not recommended as it allows MITM (man-in-the-middle) attacks - if your infrastructure is not sufficiently secured. It should only be set temporarily while you - are fixing your certificate configuration. This can be typically done by upgrading to newer - version of the operating system you run Airflow components on,by upgrading/refreshing proper - certificates in the OS or by updating certificates for your mail servers. - - If you do not set this option explicitly, it will use Airflow "email.ssl_context" configuration, - but if this configuration is not present, it will use "default" value. - type: string - version_added: 1.3.0 - example: "default" - default: ~ - templated_email_subject_path: - description: | - Allows overriding of the standard templated email subject line when the SmtpNotifier is used. - Must provide a path to the template. - type: string - version_added: 1.6.1 - example: "path/to/override/email_subject.html" - default: ~ - templated_html_content_path: - description: | - Allows overriding of the standard templated email path when the SmtpNotifier is used. Must provide - a path to the template. - type: string - version_added: 1.6.1 - example: "path/to/override/email.html" - default: ~ diff --git a/providers/tests/smtp/hooks/test_smtp.py b/providers/tests/smtp/hooks/test_smtp.py index ead0d02229b0d..e713b84423102 100644 --- a/providers/tests/smtp/hooks/test_smtp.py +++ b/providers/tests/smtp/hooks/test_smtp.py @@ -31,8 +31,6 @@ from airflow.utils import db from airflow.utils.session import create_session -from tests_common.test_utils.config import conf_vars - pytestmark = pytest.mark.db_test @@ -223,21 +221,7 @@ def test_send_mime_ssl(self, create_default_context, mock_smtp, mock_smtp_ssl): @patch("smtplib.SMTP_SSL") @patch("smtplib.SMTP") @patch("ssl.create_default_context") - def test_send_mime_ssl_none_email_context(self, create_default_context, mock_smtp, mock_smtp_ssl): - mock_smtp_ssl.return_value = Mock() - with conf_vars({("smtp", "smtp_ssl"): "True", ("email", "ssl_context"): "none"}): - with SmtpHook() as smtp_hook: - smtp_hook.send_email_smtp( - to="to", subject="subject", html_content="content", from_email="from" - ) - assert not mock_smtp.called - assert not create_default_context.called - mock_smtp_ssl.assert_called_once_with(host="smtp_server_address", port=465, timeout=30, context=None) - - @patch("smtplib.SMTP_SSL") - @patch("smtplib.SMTP") - @patch("ssl.create_default_context") - def test_send_mime_ssl_extra_context(self, create_default_context, mock_smtp, mock_smtp_ssl): + def test_send_mime_ssl_extra_none_context(self, create_default_context, mock_smtp, mock_smtp_ssl): mock_smtp_ssl.return_value = Mock() conn = Connection( conn_id="smtp_ssl_extra", @@ -246,72 +230,55 @@ def test_send_mime_ssl_extra_context(self, create_default_context, mock_smtp, mo login=None, password="None", port=465, - extra=json.dumps(dict(ssl_context="none", from_email="from")), + extra=json.dumps(dict(use_ssl=True, ssl_context="none", from_email="from")), ) db.merge_conn(conn) - with conf_vars({("smtp", "smtp_ssl"): "True", ("smtp_provider", "ssl_context"): "default"}): - with SmtpHook(smtp_conn_id="smtp_ssl_extra") as smtp_hook: - smtp_hook.send_email_smtp( - to="to", subject="subject", html_content="content", from_email="from" - ) - assert not mock_smtp.called - assert not create_default_context.called - mock_smtp_ssl.assert_called_once_with(host="smtp_server_address", port=465, timeout=30, context=None) - - @patch("smtplib.SMTP_SSL") - @patch("smtplib.SMTP") - @patch("ssl.create_default_context") - def test_send_mime_ssl_none_smtp_provider_context(self, create_default_context, mock_smtp, mock_smtp_ssl): - mock_smtp_ssl.return_value = Mock() - with conf_vars({("smtp", "smtp_ssl"): "True", ("smtp_provider", "ssl_context"): "none"}): - with SmtpHook() as smtp_hook: - smtp_hook.send_email_smtp( - to="to", subject="subject", html_content="content", from_email="from" - ) + with SmtpHook(smtp_conn_id="smtp_ssl_extra") as smtp_hook: + smtp_hook.send_email_smtp(to="to", subject="subject", html_content="content", from_email="from") assert not mock_smtp.called - assert not create_default_context.called + create_default_context.assert_not_called() mock_smtp_ssl.assert_called_once_with(host="smtp_server_address", port=465, timeout=30, context=None) @patch("smtplib.SMTP_SSL") @patch("smtplib.SMTP") @patch("ssl.create_default_context") - def test_send_mime_ssl_none_smtp_provider_default_email_context( - self, create_default_context, mock_smtp, mock_smtp_ssl - ): + def test_send_mime_ssl_extra_default_context(self, create_default_context, mock_smtp, mock_smtp_ssl): mock_smtp_ssl.return_value = Mock() - with conf_vars( - { - ("smtp", "smtp_ssl"): "True", - ("email", "ssl_context"): "default", - ("smtp_provider", "ssl_context"): "none", - } - ): - with SmtpHook() as smtp_hook: - smtp_hook.send_email_smtp( - to="to", subject="subject", html_content="content", from_email="from" - ) + conn = Connection( + conn_id="smtp_ssl_extra", + conn_type="smtp", + host="smtp_server_address", + login=None, + password="None", + port=465, + extra=json.dumps(dict(use_ssl=True, ssl_context="default", from_email="from")), + ) + db.merge_conn(conn) + with SmtpHook() as smtp_hook: + smtp_hook.send_email_smtp(to="to", subject="subject", html_content="content", from_email="from") assert not mock_smtp.called - assert not create_default_context.called - mock_smtp_ssl.assert_called_once_with(host="smtp_server_address", port=465, timeout=30, context=None) + assert create_default_context.called + mock_smtp_ssl.assert_called_once_with( + host="smtp_server_address", port=465, timeout=30, context=create_default_context.return_value + ) @patch("smtplib.SMTP_SSL") @patch("smtplib.SMTP") @patch("ssl.create_default_context") - def test_send_mime_ssl_default_smtp_provider_none_email_context( - self, create_default_context, mock_smtp, mock_smtp_ssl - ): + def test_send_mime_default_context(self, create_default_context, mock_smtp, mock_smtp_ssl): mock_smtp_ssl.return_value = Mock() - with conf_vars( - { - ("smtp", "smtp_ssl"): "True", - ("email", "ssl_context"): "none", - ("smtp_provider", "ssl_context"): "default", - } - ): - with SmtpHook() as smtp_hook: - smtp_hook.send_email_smtp( - to="to", subject="subject", html_content="content", from_email="from" - ) + conn = Connection( + conn_id="smtp_ssl_extra", + conn_type="smtp", + host="smtp_server_address", + login=None, + password="None", + port=465, + extra=json.dumps(dict(use_ssl=True, from_email="from")), + ) + db.merge_conn(conn) + with SmtpHook() as smtp_hook: + smtp_hook.send_email_smtp(to="to", subject="subject", html_content="content", from_email="from") assert not mock_smtp.called assert create_default_context.called mock_smtp_ssl.assert_called_once_with( diff --git a/providers/tests/smtp/notifications/test_smtp.py b/providers/tests/smtp/notifications/test_smtp.py index aa95f3c6f2fba..bacabf87b4d35 100644 --- a/providers/tests/smtp/notifications/test_smtp.py +++ b/providers/tests/smtp/notifications/test_smtp.py @@ -22,7 +22,6 @@ import pytest -from airflow.configuration import conf from airflow.providers.smtp.hooks.smtp import SmtpHook from airflow.providers.smtp.notifications.smtp import ( SmtpNotifier, @@ -31,7 +30,6 @@ from airflow.providers.standard.operators.empty import EmptyOperator from airflow.utils import timezone -from tests_common.test_utils.config import conf_vars from tests_common.test_utils.version_compat import AIRFLOW_V_2_10_PLUS pytestmark = pytest.mark.db_test @@ -125,14 +123,14 @@ def test_notifier_with_defaults(self, mock_smtphook_hook, create_task_instance): ti = create_task_instance(dag_id="dag", task_id="op", logical_date=timezone.datetime(2018, 1, 1)) context = {"dag": ti.dag_run.dag, "ti": ti} notifier = SmtpNotifier( - from_email=conf.get("smtp", "smtp_mail_from"), + from_email="any email", to="test_reciver@test.com", ) mock_smtphook_hook.return_value.subject_template = None mock_smtphook_hook.return_value.html_content_template = None notifier(context) mock_smtphook_hook.return_value.__enter__().send_email_smtp.assert_called_once_with( - from_email=conf.get("smtp", "smtp_mail_from"), + from_email="any email", to="test_reciver@test.com", subject="DAG dag - Task op - Run ID test in State None", html_content=mock.ANY, @@ -147,50 +145,6 @@ def test_notifier_with_defaults(self, mock_smtphook_hook, create_task_instance): content = mock_smtphook_hook.return_value.__enter__().send_email_smtp.call_args.kwargs["html_content"] assert f"{NUM_TRY} of 1" in content - @mock.patch("airflow.providers.smtp.notifications.smtp.SmtpHook") - def test_notifier_with_nondefault_conf_vars(self, mock_smtphook_hook, create_task_instance): - ti = create_task_instance(dag_id="dag", task_id="op", logical_date=timezone.datetime(2018, 1, 1)) - context = {"dag": ti.dag_run.dag, "ti": ti} - - mock_smtphook_hook.return_value.from_email = None - mock_smtphook_hook.return_value.subject_template = None - mock_smtphook_hook.return_value.html_content_template = None - - with ( - tempfile.NamedTemporaryFile(mode="wt", suffix=".txt") as f_subject, - tempfile.NamedTemporaryFile(mode="wt", suffix=".txt") as f_content, - ): - f_subject.write("Task {{ ti.task_id }} failed") - f_subject.flush() - - f_content.write("Mock content goes here") - f_content.flush() - - with conf_vars( - { - ("smtp", "templated_html_content_path"): f_content.name, - ("smtp", "templated_email_subject_path"): f_subject.name, - } - ): - notifier = SmtpNotifier( - from_email=conf.get("smtp", "smtp_mail_from"), - to="test_reciver@test.com", - ) - notifier(context) - mock_smtphook_hook.return_value.__enter__().send_email_smtp.assert_called_once_with( - from_email=conf.get("smtp", "smtp_mail_from"), - to="test_reciver@test.com", - subject="Task op failed", - html_content="Mock content goes here", - smtp_conn_id="smtp_default", - files=None, - cc=None, - bcc=None, - mime_subtype="mixed", - mime_charset="utf-8", - custom_headers=None, - ) - @mock.patch("airflow.providers.smtp.notifications.smtp.SmtpHook") def test_notifier_with_nondefault_connection_extra(self, mock_smtphook_hook, create_task_instance): ti = create_task_instance(dag_id="dag", task_id="op", logical_date=timezone.datetime(2018, 1, 1)) @@ -206,15 +160,15 @@ def test_notifier_with_nondefault_connection_extra(self, mock_smtphook_hook, cre f_content.write("Mock content goes here") f_content.flush() + mock_smtphook_hook.return_value.from_email = "{{ ti.task_id }}@test.com" mock_smtphook_hook.return_value.subject_template = f_subject.name mock_smtphook_hook.return_value.html_content_template = f_content.name notifier = SmtpNotifier( - from_email=conf.get("smtp", "smtp_mail_from"), to="test_reciver@test.com", ) notifier(context) mock_smtphook_hook.return_value.__enter__().send_email_smtp.assert_called_once_with( - from_email=conf.get("smtp", "smtp_mail_from"), + from_email="op@test.com", to="test_reciver@test.com", subject="Task op failed", html_content="Mock content goes here", diff --git a/tests_common/test_utils/version_compat.py b/tests_common/test_utils/version_compat.py index 21e7170194e36..7227de2d85962 100644 --- a/tests_common/test_utils/version_compat.py +++ b/tests_common/test_utils/version_compat.py @@ -34,3 +34,4 @@ def get_base_airflow_version_tuple() -> tuple[int, int, int]: AIRFLOW_V_2_10_PLUS = get_base_airflow_version_tuple() >= (2, 10, 0) AIRFLOW_V_3_0_PLUS = get_base_airflow_version_tuple() >= (3, 0, 0) +[].sort()