Skip to content

Commit d5ee49a

Browse files
committed
Validate task_id for mapped tasks
A regular task validates its task_id in BaseOperator.__init__, but a mapped task is built through a different path that skips __init__, so its task_id was never checked. An invalid id such as "bad*id" passed silently in .partial().expand() and in the taskflow @task mapped form, only to fail later where the id is used. Validate the id in both mapped paths so it is rejected at parse time like a regular task.
1 parent 40311ff commit d5ee49a

4 files changed

Lines changed: 35 additions & 0 deletions

File tree

‎task-sdk/src/airflow/sdk/bases/decorator.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
ListOfDictsExpandInput,
4646
is_mappable,
4747
)
48+
from airflow.sdk.definitions._internal.node import validate_key
4849
from airflow.sdk.definitions._internal.types import NOTSET
4950
from airflow.sdk.definitions.asset import Asset
5051
from airflow.sdk.definitions.context import KNOWN_CONTEXT_KEYS
@@ -621,6 +622,9 @@ def _expand(self, expand_input: ExpandInput, *, strict: bool) -> XComArg:
621622
task_id = get_unique_task_id(partial_kwargs.pop("task_id"), dag, task_group)
622623
if task_group:
623624
task_id = task_group.child_id(task_id)
625+
# Mapped tasks skip BaseOperator.__init__, so validate the id here to match a
626+
# regular task (see BaseOperator.__init__).
627+
validate_key(task_id)
624628

625629
# Logic here should be kept in sync with BaseOperatorMeta.partial().
626630
if partial_kwargs.get("wait_for_downstream"):

‎task-sdk/src/airflow/sdk/bases/operator.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,9 @@ def partial(
345345
task_group = task_group or TaskGroupContext.get_current(dag)
346346
if task_group:
347347
task_id = task_group.child_id(task_id)
348+
# Mapped tasks skip BaseOperator.__init__, so validate the id here to match a
349+
# regular task (see BaseOperator.__init__).
350+
validate_key(task_id)
348351

349352
# Merge Dag and task group level defaults into user-supplied values.
350353
dag_default_args, partial_params = get_merged_defaults(

‎task-sdk/tests/task_sdk/definitions/decorators/test_mapped.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,22 @@
1717
# under the License.
1818
from __future__ import annotations
1919

20+
import pytest
21+
2022
from airflow.sdk import DAG, TaskGroup
2123

2224

25+
def test_mapped_taskflow_task_id_is_validated():
26+
"""A mapped @task skips BaseOperator.__init__, so its id must still be validated."""
27+
28+
def f(z):
29+
pass
30+
31+
with DAG(dag_id="d", schedule=None) as dag:
32+
with pytest.raises(ValueError, match="has to be made of alphanumeric"):
33+
dag.task(task_id="bad*id")(f).expand(z=[])
34+
35+
2336
def test_mapped_task_group_id_prefix_task_id():
2437
def f(z):
2538
pass

‎task-sdk/tests/task_sdk/definitions/test_mappedoperator.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,21 @@ def test_task_mapping_with_dag():
7575
# test_task_mapping_with_dag_and_list_of_pandas_dataframe
7676

7777

78+
@pytest.mark.parametrize(
79+
("bad_task_id", "match"),
80+
[
81+
("bad*id", "has to be made of alphanumeric"),
82+
("with space", "has to be made of alphanumeric"),
83+
("a" * 251, "has to be less than 250 characters"),
84+
],
85+
)
86+
def test_mapped_task_id_is_validated(bad_task_id, match):
87+
"""A mapped task skips BaseOperator.__init__, so its id must still be validated."""
88+
with DAG("test-dag"):
89+
with pytest.raises(ValueError, match=match):
90+
MockOperator.partial(task_id=bad_task_id).expand(arg2=["a"])
91+
92+
7893
def test_task_mapping_without_dag_context():
7994
with DAG("test-dag") as dag:
8095
task1 = BaseOperator(task_id="op1")

0 commit comments

Comments
 (0)