Fix template_ext processing for Kubernetes Pod Operator

[potiuk]

The "template_ext" mechanism is useful for automatically loading and jinja-processing files which are specified in parameters of Operators. However this might lead to certain problems for example (from slack conversation):

The templated_fields in  KubernetesPodOperator seems cause the error airflow jinja2.exceptions.TemplateNotFound when some character in the column, e.g / in env_vars.
The code got error.
env_vars=[
            k8s.V1EnvVar(
                name="GOOGLE_APPLICATION_CREDENTIALS",
                value="/var/secrets/google/service-account.json",
            ),

I believe the behaviour changed comparing to not-so-long-past. Some of the changes with processing parameters with jinja recursively caused this template behaviour to be applied also nested parameters like above.

There were also several discussions and user confusion with this behaviour: #15942, #16922

There are two ways we could improve the situation:

1. limit the "template_extension" resolution to only direct string kwargs passed to the operator (I think this is no brainer and we should do it)

2. propose some "escaping" mechanism, where you could either disable template_extension processing entirely or somehow mark the parameters that should not be treated as templates.

Here I have several proposals:

a) we could add "skip_template_ext_processing" or similar parameter in BaseOperator <- I do not like it as many operators rely on this behaviour for a good reason
b) we could add "template_ext" parameter in the operator that could override the original class-level-field <- I like this one a lot
c) we could add "disable_template_ext_pattern" (str) parameter where we could specify list of regexp's where we could filter out only specific values <- this one will allow to disable template_ext much more "selectively" - only for certain parameters.

UPDATE: It only affects Kubenernetes Pod Operator due to it's recursive behaviour and should be fixed there.

[uranusjr]

In my ideal world, passing a path as an argument shouldn't need special escaping, and we should require something like

BashOperator(bash_command=Tempalte('/path/to/my-script.sh'))

to trigger templating.

More realistic though, something like

MyOperator(path=Literal('/path/to/my-file.json'))

should work?

[potiuk]

The problem is that we already have operators that heavily rely on this behaviour and will replace ANY string ending with the designated extensions with the jinja-processed value of the files they are pointing to.

This is convenient, though yeah - super-ambiguous in a number of cases, and I think we need a tactical solution for now to handle confusion it creates. I think the "recursive" behaviour is relatively new and unintended behaviour and we could address it in patchlevel release classifying it as a "bug" rather than breaking feature.

Then if we do above, I think we might even not need the "Escaping" but straight away implementing some "explicit" way of marking the template files - a variant of what you proposed @uranusjr and deprecate the usage of implicit extension handling with warning, targeting its removal for Airflow 3.

BTW - I thought Literal (https://www.python.org/dev/peps/pep-0586/) is a "type-checking" feature only, not actual class you can instantiate? Am I wrong :D ?

[uranusjr]

Oh, of course we shouldn't use typing.Literal. I was just borrowing the name and should've clarified. We should implement something like from airflow import Literal.

[kaxil]

Airflow shouldn't try to template the following. It's a bug -- for recursive we look for template_fields attribute (https://github.com/apache/airflow/pull/4743/files#diff-b5ec97b8301ab688b7e045426f24d485). k8s.V1EnvVar should have that attribute so it should not try to render template. Need to look at it in more detail 👀 .

env_vars=[
            k8s.V1EnvVar(
                name="GOOGLE_APPLICATION_CREDENTIALS",
                value="/var/secrets/google/service-account.json",
            ),

[potiuk]

But we have 'env_vars' in K8SPodOperator template_fields:

airflow/airflow/providers/cncf/kubernetes/operators/kubernetes_pod.py

Line 169 in 4dc622d

'env_vars',

So I think this behaviour is pretty expected with the current state of "recursive" behaviour.

(and I find it pretty useful to be honest)

[kaxil]

I think you are confusing 2 things:

1. Recursive classes

airflow/tests/models/test_baseoperator.py

Lines 141 to 180 in 4dc622d

	(
	# check nested fields can be templated
	ClassWithCustomAttributes(att1="{{ foo }}_1", att2="{{ foo }}_2", template_fields=["att1"]),
	{"foo": "bar"},
	ClassWithCustomAttributes(att1="bar_1", att2="{{ foo }}_2", template_fields=["att1"]),
	),
	(
	# check deep nested fields can be templated
	ClassWithCustomAttributes(
	nested1=ClassWithCustomAttributes(
	att1="{{ foo }}_1", att2="{{ foo }}_2", template_fields=["att1"]
	),
	nested2=ClassWithCustomAttributes(
	att3="{{ foo }}_3", att4="{{ foo }}_4", template_fields=["att3"]
	),
	template_fields=["nested1"],
	),
	{"foo": "bar"},
	ClassWithCustomAttributes(
	nested1=ClassWithCustomAttributes(
	att1="bar_1", att2="{{ foo }}_2", template_fields=["att1"]
	),
	nested2=ClassWithCustomAttributes(
	att3="{{ foo }}_3", att4="{{ foo }}_4", template_fields=["att3"]
	),
	template_fields=["nested1"],
	),
	),
	(
	# check null value on nested template field
	ClassWithCustomAttributes(att1=None, template_fields=["att1"]),
	{},
	ClassWithCustomAttributes(att1=None, template_fields=["att1"]),
	),
	(
	# check there is no RecursionError on circular references
	object1,
	{"foo": "bar"},
	object1,
	),

2. Iterable (list, dict, tuple, etc)

airflow/tests/models/test_baseoperator.py

Lines 121 to 123 in 4dc622d

	("{{ foo }}", {"foo": "bar"}, "bar"),
	(["{{ foo }}_1", "{{ foo }}_2"], {"foo": "bar"}, ["bar_1", "bar_2"]),
	(("{{ foo }}_1", "{{ foo }}_2"), {"foo": "bar"}, ("bar_1", "bar_2")),

Logic - Flow of code:

airflow/airflow/models/baseoperator.py

Lines 1047 to 1078 in 4dc622d

	if isinstance(content, str):
	if any(content.endswith(ext) for ext in self.template_ext):
	# Content contains a filepath
	return jinja_env.get_template(content).render(**context)
	else:
	return jinja_env.from_string(content).render(**context)
	elif isinstance(content, (XComArg, DagParam)):
	return content.resolve(context)
	if isinstance(content, tuple):
	if type(content) is not tuple:
	# Special case for named tuples
	return content.__class__(
	*(self.render_template(element, context, jinja_env) for element in content)
	)
	else:
	return tuple(self.render_template(element, context, jinja_env) for element in content)
	elif isinstance(content, list):
	return [self.render_template(element, context, jinja_env) for element in content]
	elif isinstance(content, dict):
	return {key: self.render_template(value, context, jinja_env) for key, value in content.items()}
	elif isinstance(content, set):
	return {self.render_template(element, context, jinja_env) for element in content}
	else:
	if seen_oids is None:
	seen_oids = set()
	self._render_nested_template_fields(content, context, jinja_env, seen_oids)
	return content

For nested it should do:

airflow/airflow/models/baseoperator.py

Lines 1080 to 1091 in 4dc622d

	def _render_nested_template_fields(
	self, content: Any, context: Dict, jinja_env: jinja2.Environment, seen_oids: Set
	) -> None:
	if id(content) not in seen_oids:
	seen_oids.add(id(content))
	try:
	nested_template_fields = content.template_fields
	except AttributeError:
	# content has no inner template fields
	return
	self._do_render_template_fields(content, nested_template_fields, context, jinja_env, seen_oids)

Since env_vars is a list, the items inside that list will be rendered if it is a string but since it is k8s.V1EnvVar class that does not have template_field attribute it should not.

The only reason it is doing that is we override that method for KPO:

airflow/airflow/providers/cncf/kubernetes/operators/kubernetes_pod.py

Lines 284 to 302 in 4dc622d

	def _render_nested_template_fields(
	self,
	content: Any,
	context: Dict,
	jinja_env: "jinja2.Environment",
	seen_oids: set,
	) -> None:
	if id(content) not in seen_oids and isinstance(content, k8s.V1EnvVar):
	seen_oids.add(id(content))
	self._do_render_template_fields(content, ('value', 'name'), context, jinja_env, seen_oids)
	return
	super()._render_nested_template_fields(
	content,
	context,
	jinja_env,
	seen_oids
	)

So the issue / bug is because of a fix in #14123 - and it does not affect any other operators.

[potiuk]

Ah ok. I see now :)

[potiuk]

I have changed the name to correctly reflect the discussion results.

[raphaelauv]

For people confused by the release 2.0.2

the commit Fix using XCom with ''KubernetesPodOperator'' (#17760) mostly fix the error of the templating of the K8S secrets

so don't use 2.0.1 !