diff --git a/esphome/bundle.py b/esphome/bundle.py index b6816c7c95e..efa80acc8cc 100644 --- a/esphome/bundle.py +++ b/esphome/bundle.py @@ -151,8 +151,8 @@ class ConfigBundleCreator: def __init__(self, config: dict[str, Any]) -> None: self._config = config - self._config_dir = CORE.config_dir - self._config_path = CORE.config_path + self._config_dir = Path(CORE.config_dir).resolve() + self._config_path = Path(CORE.config_path).resolve() self._files: list[BundleFile] = [] self._seen_paths: set[Path] = set() self._secrets_paths: set[Path] = set() @@ -258,21 +258,36 @@ class ConfigBundleCreator: def _discover_yaml_includes(self) -> None: """Discover YAML files loaded during config parsing. - We track files by wrapping _load_yaml_internal. The config has already - been loaded at this point (bundle is a POST_CONFIG_ACTION), so we - re-load just to discover the file list. + Deliberately uses a fresh re-parse and force-loads every deferred + ``IncludeFile`` to include *all* potentially-reachable includes, + even branches not selected by the local substitutions. Bundles are + meant to be compiled on another system where command-line + substitution overrides may choose a different branch — e.g. + ``!include network/${eth_model}/config.yaml`` must ship every + candidate so the remote build can pick any one. + + Entries with unresolved substitution variables in the filename + path are skipped with a warning (they cannot be resolved without + the substitution pass). Secrets files are tracked separately so we can filter them to only include the keys this config actually references. """ + # Must be a fresh parse: IncludeFile.load() caches its result in + # _content, and we discover files by listening for loader calls. On + # an already-parsed tree the cache is populated, .load() returns + # without calling the loader, the listener never fires, and the + # referenced files would be silently dropped from the bundle. with yaml_util.track_yaml_loads() as loaded_files: try: - yaml_util.load_yaml(self._config_path) + data = yaml_util.load_yaml(self._config_path) except EsphomeError: _LOGGER.debug( "Bundle: re-loading YAML for include discovery failed, " "proceeding with partial file list" ) + else: + _force_load_include_files(data) for fpath in loaded_files: if fpath == self._config_path.resolve(): @@ -608,6 +623,57 @@ def _add_bytes_to_tar(tar: tarfile.TarFile, name: str, data: bytes) -> None: tar.addfile(info, io.BytesIO(data)) +def _force_load_include_files(obj: Any, _seen: set[int] | None = None) -> None: + """Recursively resolve any ``IncludeFile`` instances in a YAML tree. + + Nested ``!include`` returns a deferred ``IncludeFile`` that is only + resolved during the substitution pass. During bundle discovery we need + the referenced files to actually load so the ``track_yaml_loads`` + listener fires for them. + + ``IncludeFile`` instances with unresolved substitution variables in the + filename cannot be loaded — we skip and warn about those. + """ + if _seen is None: + _seen = set() + + if isinstance(obj, yaml_util.IncludeFile): + if id(obj) in _seen: + return + _seen.add(id(obj)) + if obj.has_unresolved_expressions(): + _LOGGER.warning( + "Bundle: cannot resolve !include %s (referenced from %s) " + "with substitutions in path", + obj.file, + obj.parent_file, + ) + return + try: + loaded = obj.load() + except EsphomeError as err: + _LOGGER.warning( + "Bundle: failed to load !include %s (referenced from %s): %s", + obj.file, + obj.parent_file, + err, + ) + return + _force_load_include_files(loaded, _seen) + elif isinstance(obj, dict): + if id(obj) in _seen: + return + _seen.add(id(obj)) + for value in obj.values(): + _force_load_include_files(value, _seen) + elif isinstance(obj, (list, tuple)): + if id(obj) in _seen: + return + _seen.add(id(obj)) + for item in obj: + _force_load_include_files(item, _seen) + + def _resolve_include_path(include_path: Any) -> Path | None: """Resolve an include path to absolute, skipping system includes.""" if isinstance(include_path, str) and include_path.startswith("<"): diff --git a/tests/unit_tests/fixtures/bundle/bundle_test.yaml b/tests/unit_tests/fixtures/bundle/bundle_test.yaml index f834a8d867f..247f5cc8bb1 100644 --- a/tests/unit_tests/fixtures/bundle/bundle_test.yaml +++ b/tests/unit_tests/fixtures/bundle/bundle_test.yaml @@ -11,9 +11,9 @@ esp32: logger: <<: !include common/base.yaml -wifi: - ssid: !secret wifi_ssid - password: !secret wifi_password +# Plain nested !include — deferred as an IncludeFile until the substitution +# pass. The bundle must force-resolve it to pick up common/wifi.yaml. +wifi: !include common/wifi.yaml api: diff --git a/tests/unit_tests/fixtures/bundle/common/wifi.yaml b/tests/unit_tests/fixtures/bundle/common/wifi.yaml new file mode 100644 index 00000000000..d7e7b3cd450 --- /dev/null +++ b/tests/unit_tests/fixtures/bundle/common/wifi.yaml @@ -0,0 +1,2 @@ +ssid: !secret wifi_ssid +password: !secret wifi_password diff --git a/tests/unit_tests/test_bundle.py b/tests/unit_tests/test_bundle.py index b8b2d0ffd1a..89bf1a33b3e 100644 --- a/tests/unit_tests/test_bundle.py +++ b/tests/unit_tests/test_bundle.py @@ -5,8 +5,10 @@ from __future__ import annotations import io import json from pathlib import Path +import shutil import tarfile from typing import Any +from unittest.mock import patch import pytest @@ -20,6 +22,7 @@ from esphome.bundle import ( _add_bytes_to_tar, _default_target_dir, _find_used_secret_keys, + _force_load_include_files, extract_bundle, is_bundle_path, prepare_bundle_for_compile, @@ -485,7 +488,7 @@ def test_read_bundle_manifest_minimal(tmp_path: Path) -> None: result = read_bundle_manifest(bundle_path) assert result.esphome_version == "unknown" - assert result.files == [] + assert not result.files assert result.has_secrets is False @@ -862,6 +865,117 @@ def test_discover_files_skips_missing_directory(tmp_path: Path) -> None: assert len(files) == 1 +def test_discover_files_nested_include(tmp_path: Path) -> None: + """Nested !include files (e.g. wifi: !include wifi.yaml) are bundled.""" + config_dir = _setup_config_dir(tmp_path) + (config_dir / "test.yaml").write_text( + "esphome:\n name: test\nwifi: !include wifi.yaml\n" + ) + (config_dir / "wifi.yaml").write_text('ssid: "a"\npassword: "b"\n') + + creator = ConfigBundleCreator({}) + files = creator.discover_files() + + paths = [f.path for f in files] + assert "test.yaml" in paths + assert "wifi.yaml" in paths + + +def test_discover_files_deeply_nested_include(tmp_path: Path) -> None: + """Chains of !include (a includes b includes c) are fully resolved.""" + config_dir = _setup_config_dir(tmp_path) + (config_dir / "test.yaml").write_text( + "esphome:\n name: test\nwifi: !include level1.yaml\n" + ) + (config_dir / "level1.yaml").write_text("nested: !include level2.yaml\n") + (config_dir / "level2.yaml").write_text('value: "leaf"\n') + + creator = ConfigBundleCreator({}) + files = creator.discover_files() + + paths = [f.path for f in files] + assert "level1.yaml" in paths + assert "level2.yaml" in paths + + +def test_discover_files_nested_include_unresolved_substitution( + tmp_path: Path, +) -> None: + """!include with substitution vars in path cannot be resolved; skipped gracefully.""" + config_dir = _setup_config_dir(tmp_path) + (config_dir / "test.yaml").write_text( + "esphome:\n name: test\nwifi: !include ${platform}.yaml\n" + ) + + creator = ConfigBundleCreator({}) + # Should not raise + files = creator.discover_files() + + paths = [f.path for f in files] + assert "test.yaml" in paths + + +def test_discover_files_nested_include_load_failure( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A nested !include pointing at a missing file is logged and skipped.""" + config_dir = _setup_config_dir(tmp_path) + (config_dir / "test.yaml").write_text( + "esphome:\n name: test\nwifi: !include missing.yaml\n" + ) + + creator = ConfigBundleCreator({}) + files = creator.discover_files() + + paths = [f.path for f in files] + assert "test.yaml" in paths + assert any( + "failed to load !include" in r.message and "missing.yaml" in r.message + for r in caplog.records + ) + + +def test_force_load_skips_duplicate_include_file() -> None: + """The same IncludeFile referenced twice is only loaded once.""" + + class _StubInclude: + """Mimics yaml_util.IncludeFile minimally for _force_load testing.""" + + def __init__(self) -> None: + self.file = Path("dup.yaml") + self.parent_file = Path("root.yaml") + self.load_calls = 0 + + def has_unresolved_expressions(self) -> bool: + return False + + def load(self) -> dict[str, Any]: + self.load_calls += 1 + return {} + + stub = _StubInclude() + # Same instance appears twice — second visit must hit the _seen guard. + tree = {"a": stub, "b": [stub]} + + with patch("esphome.bundle.yaml_util.IncludeFile", _StubInclude): + _force_load_include_files(tree) + + assert stub.load_calls == 1 + + +def test_force_load_handles_cyclic_containers() -> None: + """Cyclic dict/list references don't cause infinite recursion.""" + cyclic_dict: dict[str, Any] = {} + cyclic_dict["self"] = cyclic_dict + + cyclic_list: list[Any] = [] + cyclic_list.append(cyclic_list) + + # Should return without recursing forever + _force_load_include_files(cyclic_dict) + _force_load_include_files(cyclic_list) + + def test_discover_files_yaml_reload_failure( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -1008,6 +1122,40 @@ def test_discover_files_walk_tuple_values(tmp_path: Path) -> None: assert "a.pem" in paths +# --------------------------------------------------------------------------- +# ConfigBundleCreator - fixture-based end-to-end +# --------------------------------------------------------------------------- + + +def test_discover_files_fixture_config(fixture_path: Path, tmp_path: Path) -> None: + """Use the real ``fixtures/bundle/`` tree as an end-to-end reproducer. + + The fixture config uses ``wifi: !include common/wifi.yaml`` — a plain + nested !include that is returned as a deferred ``IncludeFile`` and only + resolved during the substitution pass. Before this fix, bundle discovery + never ran substitutions, so ``common/wifi.yaml`` was silently missing + from the bundle. + """ + # Copy the fixture tree into a tmp dir so the test doesn't rely on the + # source repo being writable and so we can set CORE.config_path freely. + src = fixture_path / "bundle" + dst = tmp_path / "bundle" + shutil.copytree(src, dst) + + CORE.config_path = dst / "bundle_test.yaml" + + creator = ConfigBundleCreator({}) + files = creator.discover_files() + paths = {f.path for f in files} + + # Root and top-level !secret-referenced files + assert "bundle_test.yaml" in paths + assert "secrets.yaml" in paths + # The nested !include — this is what regressed when IncludeFile became + # deferred (PR #12213). + assert "common/wifi.yaml" in paths + + # --------------------------------------------------------------------------- # ConfigBundleCreator - create_bundle # ---------------------------------------------------------------------------