Skip to content

Commit

Permalink
BREAKING CHANGE: replace Airflow config by conx extras in SMTP provider
Browse files Browse the repository at this point in the history
  • Loading branch information
hussein-awala committed Jan 28, 2025
1 parent 4db60c1 commit 9bf0bbe
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 189 deletions.
2 changes: 1 addition & 1 deletion dev/breeze/doc/images/output_k8s_run-complete-tests.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1855cee910a2f7039dae5d4af44399a6
eb44c30eb7a68dbe7a20d059a30be809
2 changes: 1 addition & 1 deletion generated/provider_dependencies.json
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@
"deps": [
"apache-airflow>=2.9.0",
"dnspython>=1.13.0",
"pymongo>=4.0.0"
"pymongo>=4.0.0,!=4.11"
],
"devel-deps": [
"mongomock>=4.0.0"
Expand Down
22 changes: 22 additions & 0 deletions providers/src/airflow/providers/smtp/CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
.....

Expand Down
22 changes: 8 additions & 14 deletions providers/src/airflow/providers/smtp/hooks/smtp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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:
"""
Expand Down
29 changes: 14 additions & 15 deletions providers/src/airflow/providers/smtp/notifications/smtp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -110,28 +107,30 @@ 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:
smtp_default_templated_html_content_path: str
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:
Expand Down
40 changes: 0 additions & 40 deletions providers/src/airflow/providers/smtp/provider.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: ~
103 changes: 35 additions & 68 deletions providers/tests/smtp/hooks/test_smtp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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",
Expand All @@ -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(
Expand Down
Loading

0 comments on commit 9bf0bbe

Please sign in to comment.