Cleanup committed zip file in tests/dags

.gitignore

@@ -257,3 +257,6 @@ airflow-build-dockerfile*
 
 # Temporary ignore uv.lock until we integrate it fully in our constraint preparation mechanism
 /uv.lock
+
+# Ignore zip files https://github.com/apache/airflow/issues/46449
+*.zip

.pre-commit-config.yaml

@@ -1088,6 +1088,14 @@ repos:
         pass_filenames: true
         files: \.py$
         additional_dependencies: ['rich>=12.4.4']
+      - id: check-zip-file-is-not-committed
+        name: Check no zip files are committed
+        description: Zip files are not allowed in the repository
+        language: fail
+        entry: |
+          Zip files are not allowed in the repository as they are hard to
+          track and have security implications. Please remove the zip file from the repository.
+        files: \.zip$
       - id: check-code-deprecations
         name: Check deprecations categories in decorators
         entry: ./scripts/ci/pre_commit/check_deprecations.py

contributing-docs/08_static_code_checks.rst

@@ -252,6 +252,8 @@ require Breeze Docker image to be built locally.
 +-----------------------------------------------------------+--------------------------------------------------------+---------+
 | check-xml                                                 | Check XML files with xmllint                           |         |
 +-----------------------------------------------------------+--------------------------------------------------------+---------+
+| check-zip-file-is-not-committed                           | Check no zip files are committed                       |         |
++-----------------------------------------------------------+--------------------------------------------------------+---------+
 | codespell                                                 | Run codespell                                          |         |
 +-----------------------------------------------------------+--------------------------------------------------------+---------+
 | compile-ui-assets                                         | Compile ui assets (manual)                             |         |

dev/breeze/doc/images/output_static-checks.txt

@@ -1 +1 @@
-bc496446ce0ed673262a2515daf88da9
+16b7f3c6c23d11208195ebda9a569fe4

dev/breeze/src/airflow_breeze/pre_commit_ids.py

@@ -91,6 +91,7 @@
     "check-urlparse-usage-in-code",
     "check-usage-of-re2-over-re",
     "check-xml",
+    "check-zip-file-is-not-committed",
     "codespell",
     "compile-ui-assets",
     "compile-ui-assets-dev",

scripts/ci/pre_commit/check_init_in_tests.py

@@ -42,7 +42,9 @@
 if __name__ == "__main__":
     for dirname, sub_dirs, _ in os.walk(ROOT_DIR / "tests"):
         dir = Path(dirname)
-        sub_dirs[:] = [subdir for subdir in sub_dirs if subdir not in {"__pycache__", "test_logs"}]
+        sub_dirs[:] = [
+            subdir for subdir in sub_dirs if subdir not in {"__pycache__", "test_logs", "test_zip"}
+        ]
         for sub_dir in sub_dirs:
             init_py_path = dir / sub_dir / "__init__.py"
             if not init_py_path.exists():

tests/conftest.py

@@ -20,16 +20,15 @@
 import logging
 import os
 import sys
+import zipfile
 from contextlib import contextmanager
+from pathlib import Path
 from typing import TYPE_CHECKING
 
 import pytest
 
 from tests_common.test_utils.log_handlers import non_pytest_handlers
 
-if TYPE_CHECKING:
-    from pathlib import Path
-
 # We should set these before loading _any_ of the rest of airflow so that the
 # unit test mode config is set as early as possible.
 assert "airflow" not in sys.modules, "No airflow module can be imported before these lines"
@@ -133,6 +132,19 @@ def _config_bundle(path_to_parse: Path | str):
     return _config_bundle
 
 
+@pytest.fixture
+def test_zip_path(tmp_path: Path):
+    TEST_DAGS_FOLDER = Path(__file__).parent / "dags"
+    test_zip_folder = TEST_DAGS_FOLDER / "test_zip"
+    zipped = tmp_path / "test_zip.zip"
+    with zipfile.ZipFile(zipped, "w") as zf:
+        for root, _, files in os.walk(test_zip_folder):
+            for file in files:
+                zf.write(os.path.join(root, file), os.path.relpath(os.path.join(root, file), test_zip_folder))
+
+    return os.fspath(zipped)
+
+
 if TYPE_CHECKING:
     # Static checkers do not know about pytest fixtures' types and return,
     # In case if them distributed through third party packages.

tests/dag_processing/test_manager.py

@@ -623,17 +623,16 @@ def test_send_file_processing_statsd_timing(
         )
 
     def test_refresh_dags_dir_doesnt_delete_zipped_dags(
-        self, tmp_path, testing_dag_bundle, configure_testing_dag_bundle
+        self, tmp_path, testing_dag_bundle, configure_testing_dag_bundle, test_zip_path
     ):
         """Test DagFileProcessorManager._refresh_dag_dir method"""
         dagbag = DagBag(dag_folder=tmp_path, include_examples=False)
-        zipped_dag_path = os.path.join(TEST_DAGS_FOLDER, "test_zip.zip")
-        dagbag.process_file(zipped_dag_path)
+        dagbag.process_file(test_zip_path)
         dag = dagbag.get_dag("test_zip_dag")
         DAG.bulk_write_to_db("testing", None, [dag])
         SerializedDagModel.write_dag(dag, bundle_name="testing")
 
-        with configure_testing_dag_bundle(zipped_dag_path):
+        with configure_testing_dag_bundle(test_zip_path):
             manager = DagFileProcessorManager(max_runs=1)
             manager.run()
 
@@ -646,12 +645,12 @@ def test_refresh_dags_dir_doesnt_delete_zipped_dags(
 
     @pytest.mark.usefixtures("testing_dag_bundle")
     def test_refresh_dags_dir_deactivates_deleted_zipped_dags(
-        self, session, tmp_path, configure_testing_dag_bundle
+        self, session, tmp_path, configure_testing_dag_bundle, test_zip_path
     ):
         """Test DagFileProcessorManager._refresh_dag_dir method"""
         dag_id = "test_zip_dag"
         filename = "test_zip.zip"
-        source_location = os.path.join(TEST_DAGS_FOLDER, filename)
+        source_location = test_zip_path
         bundle_path = Path(tmp_path, "test_refresh_dags_dir_deactivates_deleted_zipped_dags")
         bundle_path.mkdir(exist_ok=True)
         zip_dag_path = bundle_path / filename

tests/dags/test_zip/file_no_airflow_dag.py

@@ -0,0 +1,16 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.

tests/dags/test_zip/test_zip.py

@@ -0,0 +1,33 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import annotations
+
+from datetime import datetime
+
+from airflow.models import DAG
+from airflow.providers.standard.operators.empty import EmptyOperator
+
+DEFAULT_DATE = datetime(2030, 1, 1)
+
+# DAG tests backfill with pooled tasks
+# Previously backfill would queue the task but never run it
+dag1 = DAG(dag_id="test_zip_dag", start_date=DEFAULT_DATE, schedule="@daily")
+dag1_task1 = EmptyOperator(task_id="dummy", dag=dag1, owner="airflow")
+
+with DAG(dag_id="test_zip_autoregister", schedule=None, start_date=DEFAULT_DATE):
+    EmptyOperator(task_id="noop")

tests/dags/test_zip/test_zip_module/__init__.py

@@ -0,0 +1,16 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.

tests/dags/test_zip/test_zip_module/test.py

@@ -0,0 +1,21 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+
+def say_hello():
+    print("Hello")

tests/models/test_dagbag.py

@@ -191,13 +191,12 @@ def my_flow():
         )
         assert dagbag.dags == dags_in_bag  # Should not change.
 
-    def test_zip_skip_log(self, caplog):
+    def test_zip_skip_log(self, caplog, test_zip_path):
         """
         test the loading of a DAG from within a zip file that skips another file because
         it doesn't have "airflow" and "DAG"
         """
         caplog.set_level(logging.INFO)
-        test_zip_path = os.path.join(TEST_DAGS_FOLDER, "test_zip.zip")
         dagbag = DagBag(dag_folder=test_zip_path, include_examples=False)
 
         assert dagbag.has_logged
@@ -206,13 +205,13 @@ def test_zip_skip_log(self, caplog):
             "assumed to contain no DAGs. Skipping." in caplog.text
         )
 
-    def test_zip(self, tmp_path):
+    def test_zip(self, tmp_path, test_zip_path):
         """
         test the loading of a DAG within a zip file that includes dependencies
         """
         syspath_before = deepcopy(sys.path)
         dagbag = DagBag(dag_folder=os.fspath(tmp_path), include_examples=False)
-        dagbag.process_file(os.path.join(TEST_DAGS_FOLDER, "test_zip.zip"))
+        dagbag.process_file(test_zip_path)
         assert dagbag.get_dag("test_zip_dag")
         assert sys.path == syspath_before  # sys.path doesn't change
         assert not dagbag.import_errors
@@ -357,14 +356,6 @@ def process_file(self, filepath, only_if_updated=True, safe_mode=True):
     @pytest.mark.parametrize(
         ("file_to_load", "expected"),
         (
-            pytest.param(
-                TEST_DAGS_FOLDER / "test_zip.zip",
-                {
-                    "test_zip_dag": "dags/test_zip.zip/test_zip.py",
-                    "test_zip_autoregister": "dags/test_zip.zip/test_zip.py",
-                },
-                id="test_zip.zip",
-            ),
             pytest.param(
                 pathlib.Path(example_dags_folder) / "example_bash_operator.py",
                 {"example_bash_operator": "airflow/example_dags/example_bash_operator.py"},
@@ -380,6 +371,26 @@ def test_get_dag_registration(self, file_to_load, expected):
             assert dag, f"{dag_id} was bagged"
             assert dag.fileloc.endswith(path)
 
+    @pytest.mark.parametrize(
+        ("expected"),
+        (
+            pytest.param(
+                {
+                    "test_zip_dag": "test_zip.zip/test_zip.py",
+                    "test_zip_autoregister": "test_zip.zip/test_zip.py",
+                },
+                id="test_zip.zip",
+            ),
+        ),
+    )
+    def test_get_zip_dag_registration(self, test_zip_path, expected):
+        dagbag = DagBag(dag_folder=os.devnull, include_examples=False)
+        dagbag.process_file(test_zip_path)
+        for dag_id, path in expected.items():
+            dag = dagbag.get_dag(dag_id)
+            assert dag, f"{dag_id} was bagged"
+            assert dag.fileloc.endswith(f"{pathlib.Path(test_zip_path).parent}/{path}")
+
     def test_dag_registration_with_failure(self):
         dagbag = DagBag(dag_folder=os.devnull, include_examples=False)
         found = dagbag.process_file(str(TEST_DAGS_FOLDER / "test_invalid_dup_task.py"))
@@ -431,12 +442,12 @@ def process_file(self, filepath, only_if_updated=True, safe_mode=True):
         assert dagbag.process_file_calls == 2
 
     @patch.object(DagModel, "get_current")
-    def test_refresh_packaged_dag(self, mock_dagmodel):
+    def test_refresh_packaged_dag(self, mock_dagmodel, test_zip_path):
         """
         Test that we can refresh a packaged DAG
         """
         dag_id = "test_zip_dag"
-        fileloc = os.path.realpath(os.path.join(TEST_DAGS_FOLDER, "test_zip.zip/test_zip.py"))
+        fileloc = os.path.realpath(os.path.join(test_zip_path, "test_zip.py"))
 
         mock_dagmodel.return_value = DagModel()
         mock_dagmodel.return_value.last_expired = datetime.max.replace(tzinfo=timezone.utc)
@@ -450,7 +461,7 @@ def process_file(self, filepath, only_if_updated=True, safe_mode=True):
                     _TestDagBag.process_file_calls += 1
                 return super().process_file(filepath, only_if_updated, safe_mode)
 
-        dagbag = _TestDagBag(dag_folder=os.path.realpath(TEST_DAGS_FOLDER), include_examples=False)
+        dagbag = _TestDagBag(dag_folder=os.path.realpath(test_zip_path), include_examples=False)
 
         assert dagbag.process_file_calls == 1
         dag = dagbag.get_dag(dag_id)
@@ -877,13 +888,20 @@ def test_dabgag_captured_warnings(self):
             assert dag_file not in dagbag.captured_warnings
             assert dagbag.dagbag_stats[0].warning_num == 0
 
-    def test_dabgag_captured_warnings_zip(self):
-        dag_file = os.path.join(TEST_DAGS_FOLDER, "test_dag_warnings.zip")
-        in_zip_dag_file = f"{dag_file}/test_dag_warnings.py"
-        dagbag = DagBag(dag_folder=dag_file, include_examples=False)
+    @pytest.fixture
+    def warning_zipped_dag_path(self, tmp_path: pathlib.Path) -> str:
+        warnings_dag_file = TEST_DAGS_FOLDER / "test_dag_warnings.py"
+        zipped = tmp_path / "test_dag_warnings.zip"
+        with zipfile.ZipFile(zipped, "w") as zf:
+            zf.write(warnings_dag_file, warnings_dag_file.name)
+        return os.fspath(zipped)
+
+    def test_dabgag_captured_warnings_zip(self, warning_zipped_dag_path: str):
+        in_zip_dag_file = f"{warning_zipped_dag_path}/test_dag_warnings.py"
+        dagbag = DagBag(dag_folder=warning_zipped_dag_path, include_examples=False)
         assert len(dagbag.dag_ids) == 1
-        assert dag_file in dagbag.captured_warnings
-        captured_warnings = dagbag.captured_warnings[dag_file]
+        assert warning_zipped_dag_path in dagbag.captured_warnings
+        captured_warnings = dagbag.captured_warnings[warning_zipped_dag_path]
         assert len(captured_warnings) == 2
         assert captured_warnings[0] == (f"{in_zip_dag_file}:47: DeprecationWarning: Deprecated Parameter")
         assert captured_warnings[1] == f"{in_zip_dag_file}:49: UserWarning: Some Warning"

tests/utils/test_file.py

@@ -88,8 +88,8 @@ def test_open_maybe_zipped_normal_file_with_zip_in_name(self):
             open_maybe_zipped(path)
             mock_file.assert_called_once_with(path, mode="r")
 
-    def test_open_maybe_zipped_archive(self):
-        test_file_path = os.path.join(TEST_DAGS_FOLDER, "test_zip.zip", "test_zip.py")
+    def test_open_maybe_zipped_archive(self, test_zip_path):
+        test_file_path = os.path.join(test_zip_path, "test_zip.py")
         with open_maybe_zipped(test_file_path, "r") as test_file:
             content = test_file.read()
         assert isinstance(content, str)
@@ -219,7 +219,7 @@ def test_get_modules_from_invalid_file(self):
 
         assert len(modules) == 0
 
-    def test_list_py_file_paths(self):
+    def test_list_py_file_paths(self, test_zip_path):
         detected_files = set()
         expected_files = set()
         # No_dags is empty, _invalid_ is ignored by .airflowignore
@@ -234,6 +234,8 @@ def test_list_py_file_paths(self):
             "test_invalid_param4.py",
             "test_nested_dag.py",
             "test_imports.py",
+            "file_no_airflow_dag.py",  # no_dag test case in test_zip folder
+            "test.py",  # no_dag test case in test_zip_module folder
             "__init__.py",
         }
         for root, _, files in os.walk(TEST_DAG_FOLDER):