jason810496 opened a new issue, #73800:
URL: https://github.com/apache/airflow/issues/73800
A Dag declared in a language SDK is serialized by that SDK and handed to
Airflow as serialized-Dag JSON. Nothing on that path checks its dag id, task
ids or task-group ids, because the checks live in the Python object
constructors the path never reaches:
- `validate_key(k, max_length=250)` with `KEY_REGEX = ^[\w.-]+$`, called
from `BaseOperator.__init__` (`task-sdk/src/airflow/sdk/bases/operator.py`) and
the `DAG.dag_id` validator (`task-sdk/src/airflow/sdk/definitions/dag.py`).
- `validate_group_key(k, max_length=200)` with `GROUP_KEY_REGEX = ^[\w-]+$`,
for task groups.
- The `..` rule, gated on `core.allow_double_dot_in_ids`, in
`airflow-core/src/airflow/utils/helpers.py`. It guards conn ids and run ids
rather than task ids.
So an id that Python would reject can reach the metadata DB from a language
SDK. The columns are `StringID()`, that is `String(250)`, so an over-long id
surfaces as a database error rather than as a report against the file that
produced it.
Every SDK has had to compensate on its own, and each one can only warn:
| | length | characters | `..` |
|---|---|---|---|
| Python | 250, enforced at construction | `^[\w.-]+$`, enforced | conn and
run ids only |
| Go | 250, warning at pack time | `^[\p{L}\p{N}_.-]+$`, warning | warning |
| TypeScript | 250, warning at pack time | warning at pack time, and
enforced at declaration | warning, and unreachable from a declared id |
| Java | nothing yet, added by #69937 | same | same |
That is three independent copies of one rule set:
`go-sdk/cmd/airflow-go-pack/validate.go`, `ts-sdk/src/cli/validate.ts`, and the
`checkAirflowBundle` Gradle task in #69937. All three carry the same comment,
that the server validates authoritatively and a packer cannot see
`core.allow_double_dot_in_ids`. On this path the server does not validate at
all, so the comment describes a guarantee that is missing.
### What to do
Validate ids in the Dag processing stage, where a parsing result becomes a
Dag and an import error can be recorded against the file that produced it, the
way any other parse failure is. That covers every producer with one
implementation, whatever language wrote the Dag, and it is the only place the
config is readable. The structural checks a serialized Dag is gaining at that
same point, such as cycles and edges naming tasks that do not exist, are the
natural neighbours.
Keep the SDK-side checks. They report at declaration or at build time, with
the author's own file and line, which a server-side error cannot give. They are
the fast feedback, not the guarantee.
### Acceptance criteria
- A serialized Dag whose dag id, task id or group id breaks the length,
character or `..` rules is rejected in Dag processing, with an import error
against the file.
- One rule set, shared by Python Dags and language-SDK Dags,
`core.allow_double_dot_in_ids` included.
- Each language SDK keeps checking what it can locally, and the three packer
implementations agree with the server's rules.
- Conformance coverage for a rejected id in each SDK.
Raised in review on https://github.com/apache/airflow/pull/73437, and
related to #69937.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
