apache / airflow

Apache Airflow - A platform to programmatically author, schedule, and monitor workflows
https://airflow.apache.org/
Apache License 2.0
37.1k stars 14.3k forks source link

XCom and Log views broken for DAG runs with `/` in run id #42679

Open jkramer-ginkgo opened 1 month ago

jkramer-ginkgo commented 1 month ago

Apache Airflow version

Other Airflow 2 version (please specify below)

If "Other Airflow 2 version" selected, which one?

2.10.0

What happened?

The UI does not work for Xcom and Log views for DAG runs with / in the run id. There are 404s from the page for API calls like https://<airflow>/api/v1/dags/<dag id>/dagRuns/<run id>. The <run id> is not url encoded so a slash in there certainly screws up this API call.

What you think should happen instead?

No response

How to reproduce

Create a DAG run with a / character in it and try to view the logs in the UI.

Operating System

Linux (Ubuntu 22.04)

Versions of Apache Airflow Providers

No response

Deployment

Docker-Compose

Deployment details

No response

Anything else?

No response

Are you willing to submit PR?

Code of Conduct

jscheffl commented 1 month ago

As we have excluded the / from the standard run-id pattern in config allowed_run_id_pattern this might be a un-tested side effect. So probably I'd recommend to limit and change your run-ids to use safe characters. As we are only maintaining the 2.10 version until a totally new UI is implemented, I assume it needs a volunteer (or your?) contribution to have a fix.

josix commented 1 month ago

🙋🏻‍♂️ I can take this issue

josix commented 1 month ago

Hi @jkramer-ginkgo, could you elaborate on how you triggered a DAG run with a '/' in its id? I tried to reproduce it, but Airflow seems to have a verification process in place. It shows that a dagrun ID containing a slash, created via dag.create_dagrun, is prohibited

jkramer-ginkgo commented 1 month ago

@josix We have allowed_run_id_pattern = .*. Given it's a user config, I imagine it should be assumed URL-escapable chars can be expected in the run ids.

jkramer-ginkgo commented 1 month ago

I appreciate you taking a look into this