bujjibabukatta opened a new pull request, #71166:
URL: https://github.com/apache/airflow/pull/71166

   DAG processor treats any zip-format file (.jar, .pptx, .docx, .xlsx, etc.) 
as a potential DAG bundle, not just .zip
   
   ## Problem
   
   `find_dag_file_paths` (`airflow-core/src/airflow/utils/file.py`) decided 
whether to attempt
   DAG discovery on a file using `zipfile.is_zipfile()` alone — a content sniff 
for the PK zip
   magic bytes, not an extension check. The same unguarded pattern existed in
   `PythonDagImporter.list_dag_files` 
(`airflow-core/src/airflow/dag_processing/importers/python_importer.py`),
   the newer importer-based discovery path used by `DagBag`.
   
   Since the zip container format underlies many common file types beyond 
`.zip` itself —
   `.jar`, `.pptx`, `.docx`, `.xlsx`, `.apk`, `.epub`, `.odt`, `.whl`, etc. — 
any of these
   dropped into a DAGs folder (a build artifact, a supporting doc, a packaged 
dependency) would
   pass this check and get opened and scanned via `might_contain_dag`, purely 
because it shares
   the underlying container format with Airflow's own zipped-DAG-bundle 
feature. At minimum this
   is wasted work on every DAG processor cycle; depending on the archive's 
contents it can also
   produce confusing log noise.
   
   This was reported previously in #45718 with a `.pptx` file, but that issue 
was closed as
   invalid because the specific symptom reported there turned out to be an 
unrelated bug. The
   underlying zip-detection design issue itself was never fixed.
   
   ## Fix
   
   Gate both zip-bundle branches on `path.suffix == ".zip"` in addition to the 
existing content
   sniff, so only files actually named `.zip` are treated as DAG zip bundles:
   
   - `airflow-core/src/airflow/utils/file.py` — `find_dag_file_paths`
   - `airflow-core/src/airflow/dag_processing/importers/python_importer.py` — 
`PythonDagImporter.list_dag_files`
   
   I traced every other `zipfile.is_zipfile()` call site in the DAG processing 
code
   (`dag_processing/manager.py`'s `_get_observed_filelocs`, 
`python_importer.py`'s `import_file`,
   and `utils/file.py`'s `correct_maybe_zipped`/`open_maybe_zipped`) and 
confirmed each is only
   ever reached after this extension check upstream (or is already gated by a 
`.zip`-anchored
   regex), so no other locations required changes.
   
   ## Tests
   
   Added a regression test for each fixed code path, each building a real 
zip-format file named
   `.jar` (via `zipfile.ZipFile`, so it genuinely sniffs as a zip) alongside a 
`.py` file and a
   correctly-named `.zip` bundle, and asserting the `.jar` is excluded while 
the other two are
   still discovered:
   
   - 
`airflow-core/tests/unit/utils/test_file.py::TestListPyFilesPath::test_list_py_file_paths_ignores_non_zip_zip_format_files`
   - 
`airflow-core/tests/unit/dag_processing/importers/test_python_importer.py::TestPythonDagImporterListDagFiles::test_list_dag_files_ignores_non_zip_zip_format_files`
 (new file)
   
   Also added `airflow-core/newsfragments/71125.bugfix.rst`.
   
   closes: #71125
   
   ---
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes
   
   Generated-by: Claude (Anthropic), used to draft the code fix, the two 
regression test cases,
   this PR description, and the newsfragment, following [the 
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions).
   All generated code and tests were reviewed and verified by me before 
submission.


-- 
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]

Reply via email to