Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not silently allow the use of undefined variables in jinja2 templates #11016

Merged
merged 1 commit into from
Sep 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,20 @@ with third party services to the ``airflow.providers`` package.
All changes made are backward compatible, but if you use the old import paths you will
see a deprecation warning. The old import paths can be abandoned in the future.

#### Change to undefined variable handling in templates

Prior to Airflow 2.0 Jinja Templates would permit the use of undefined variables. They would render as an
empty string, with no indication to the user an undefined variable was used. With this release, any template
rendering involving undefined variables will fail the task, as well as displaying an error in the UI when
rendering.

The behavior can be reverted when instantiating a DAG.
```python
import jinja2

dag = DAG('simple_dag', template_undefined=jinja2.Undefined)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it also possible on a template-by-template basis to do it as {{ something | default "x" }}?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I believe all standard jinja2 templating features are still available.

```

### Breaking Change in OAuth

The flask-ouathlib has been replaced with authlib because flask-outhlib has
Expand Down
4 changes: 2 additions & 2 deletions airflow/models/dag.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ class DAG(BaseDag, LoggingMixin):
default
:type template_searchpath: str or list[str]
:param template_undefined: Template undefined type.
:type template_undefined: jinja2.Undefined
:type template_undefined: jinja2.StrictUndefined
:param user_defined_macros: a dictionary of macros that will be exposed
in your jinja templates. For example, passing ``dict(foo='bar')``
to this argument allows you to ``{{ foo }}`` in all jinja
Expand Down Expand Up @@ -224,7 +224,7 @@ def __init__(
end_date: Optional[datetime] = None,
full_filepath: Optional[str] = None,
template_searchpath: Optional[Union[str, Iterable[str]]] = None,
template_undefined: Type[jinja2.Undefined] = jinja2.Undefined,
template_undefined: Type[jinja2.StrictUndefined] = jinja2.StrictUndefined,
user_defined_macros: Optional[Dict] = None,
user_defined_filters: Optional[Dict] = None,
default_args: Optional[Dict] = None,
Expand Down
16 changes: 15 additions & 1 deletion tests/models/test_baseoperator.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ class TestBaseOperator(unittest.TestCase):
@parameterized.expand(
[
("{{ foo }}", {"foo": "bar"}, "bar"),
("{{ foo }}", {}, ""),
(["{{ foo }}_1", "{{ foo }}_2"], {"foo": "bar"}, ["bar_1", "bar_2"]),
(("{{ foo }}_1", "{{ foo }}_2"), {"foo": "bar"}, ("bar_1", "bar_2")),
(
Expand Down Expand Up @@ -184,6 +183,14 @@ def test_render_template_fields_no_change(self, content):
result = task.render_template(content, {"foo": "bar"})
self.assertEqual(content, result)

def test_render_template_field_undefined_default(self):
"""Test render_template with template_undefined unchanged."""
with DAG("test-dag", start_date=DEFAULT_DATE):
task = DummyOperator(task_id="op1")

with self.assertRaises(jinja2.UndefinedError):
task.render_template("{{ foo }}", {})

def test_render_template_field_undefined_strict(self):
"""Test render_template with template_undefined configured."""
with DAG("test-dag", start_date=DEFAULT_DATE, template_undefined=jinja2.StrictUndefined):
Expand All @@ -192,6 +199,13 @@ def test_render_template_field_undefined_strict(self):
with self.assertRaises(jinja2.UndefinedError):
task.render_template("{{ foo }}", {})

def test_render_template_field_undefined_not_strict(self):
"""Test render_template with template_undefined configured to silently error."""
with DAG("test-dag", start_date=DEFAULT_DATE, template_undefined=jinja2.Undefined):
task = DummyOperator(task_id="op1")

self.assertEqual(task.render_template("{{ foo }}", {}), "")

def test_nested_template_fields_declared_must_exist(self):
"""Test render_template when a nested template field is missing."""
with DAG("test-dag", start_date=DEFAULT_DATE):
Expand Down