mirror of
https://github.com/esphome/esphome.git
synced 2026-10-15 02:32:49 +00:00
[core] Report incomplete YAML discovery and record remote package sources
This commit is contained in:
@@ -418,6 +418,18 @@ class ConfigBundleCreator:
|
||||
must ship every candidate so the remote build can pick any one.
|
||||
"""
|
||||
discovered = yaml_util.discover_user_yaml_files(self._config_path)
|
||||
if discovered.load_errors:
|
||||
_LOGGER.warning(
|
||||
"Bundle may be incomplete; could not load all configuration files: %s",
|
||||
"; ".join(discovered.load_errors),
|
||||
)
|
||||
if discovered.unresolved:
|
||||
_LOGGER.warning(
|
||||
"Bundle may be incomplete; %d !include path(s) use "
|
||||
"substitutions and cannot be captured: %s",
|
||||
len(discovered.unresolved),
|
||||
", ".join(discovered.unresolved),
|
||||
)
|
||||
self._secrets_paths.update(discovered.secrets)
|
||||
# A !secret inside a file this re-parse does not reach (for example
|
||||
# a git-fetched package the builder re-fetches) still resolves
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
from collections import UserDict
|
||||
from collections.abc import Callable
|
||||
from dataclasses import dataclass, field
|
||||
from functools import reduce
|
||||
import logging
|
||||
from pathlib import Path
|
||||
@@ -34,11 +35,46 @@ from esphome.const import (
|
||||
CONF_VARS,
|
||||
__version__ as ESPHOME_VERSION,
|
||||
)
|
||||
from esphome.core import EsphomeError
|
||||
from esphome.core import CORE, EsphomeError
|
||||
|
||||
_LOGGER = logging.getLogger(__name__)
|
||||
|
||||
DOMAIN = CONF_PACKAGES
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class RemotePackageSource:
|
||||
"""A remote source a package was fetched from while processing the config."""
|
||||
|
||||
url: str
|
||||
ref: str | None
|
||||
|
||||
|
||||
@dataclass
|
||||
class PackagesData:
|
||||
"""Per-run package state, keyed under DOMAIN in CORE.data."""
|
||||
|
||||
# Insertion-ordered and de-duplicated
|
||||
remote_sources: dict[RemotePackageSource, None] = field(default_factory=dict)
|
||||
|
||||
|
||||
def _get_data() -> PackagesData:
|
||||
if DOMAIN not in CORE.data:
|
||||
CORE.data[DOMAIN] = PackagesData()
|
||||
return CORE.data[DOMAIN]
|
||||
|
||||
|
||||
def get_remote_package_sources() -> tuple[RemotePackageSource, ...]:
|
||||
"""Remote sources fetched while processing this config, in fetch order.
|
||||
|
||||
Consumers (e.g. store_yaml) use this to tell which parts of the config
|
||||
came from remote repositories rather than local files.
|
||||
"""
|
||||
if (data := CORE.data.get(DOMAIN)) is None:
|
||||
return ()
|
||||
return tuple(data.remote_sources)
|
||||
|
||||
|
||||
# Guard against infinite include chains (e.g. A includes B includes A).
|
||||
MAX_INCLUDE_DEPTH = 20
|
||||
|
||||
@@ -192,6 +228,9 @@ def _process_remote_package(config: dict[str, Any]) -> dict[str, Any]:
|
||||
username=config.get(CONF_USERNAME),
|
||||
password=config.get(CONF_PASSWORD),
|
||||
)
|
||||
_get_data().remote_sources[
|
||||
RemotePackageSource(config[CONF_URL], config.get(CONF_REF))
|
||||
] = None
|
||||
files: list[dict[str, Any]] = []
|
||||
|
||||
# ``repo_root`` is the directory containing ``.git`` and must be passed
|
||||
|
||||
+59
-17
@@ -12,7 +12,7 @@ import math
|
||||
import os
|
||||
from pathlib import Path
|
||||
import re
|
||||
from typing import Any
|
||||
from typing import Any, NamedTuple
|
||||
import uuid
|
||||
|
||||
import yaml
|
||||
@@ -394,6 +394,7 @@ def _load_include_candidates(
|
||||
seen: set[int],
|
||||
expanded_paths: set[Path],
|
||||
keepalive: list[Any],
|
||||
result: ForceLoadResult,
|
||||
) -> None:
|
||||
"""Load every filesystem candidate for an unresolved ``IncludeFile``."""
|
||||
from voluptuous import Invalid
|
||||
@@ -406,6 +407,7 @@ def _load_include_candidates(
|
||||
include.file,
|
||||
include.parent_file,
|
||||
)
|
||||
result.unresolved.append(str(include.file))
|
||||
return
|
||||
_LOGGER.debug(
|
||||
"Expanding !include %s (referenced from %s) to %d candidate file(s)",
|
||||
@@ -431,16 +433,18 @@ def _load_include_candidates(
|
||||
include.file,
|
||||
err,
|
||||
)
|
||||
result.errors.append(f"{candidate}: {err}")
|
||||
continue
|
||||
# The throwaway IncludeFile is this tree's only owner; keep the tree
|
||||
# alive so ids recorded in ``seen`` stay unique for the traversal.
|
||||
keepalive.append(loaded)
|
||||
force_load_include_files(
|
||||
_force_load_include_files(
|
||||
loaded,
|
||||
warn_on_unresolved=warn_on_unresolved,
|
||||
_seen=seen,
|
||||
_expanded_paths=expanded_paths,
|
||||
_keepalive=keepalive,
|
||||
_result=result,
|
||||
)
|
||||
|
||||
|
||||
@@ -458,14 +462,21 @@ def find_secret_references(text: str) -> set[str]:
|
||||
return {match.group(1) for match in _SECRET_REFERENCE_RE.finditer(text)}
|
||||
|
||||
|
||||
class ForceLoadResult(NamedTuple):
|
||||
"""Outcome of :func:`force_load_include_files`.
|
||||
|
||||
``unresolved`` lists ``!include`` path strings with substitution variables or
|
||||
expressions that matched no candidate file; ``errors`` lists includes that failed
|
||||
to load. Either being non-empty means the walk was incomplete.
|
||||
"""
|
||||
|
||||
unresolved: list[str]
|
||||
errors: list[str]
|
||||
|
||||
|
||||
def force_load_include_files(
|
||||
obj: Any,
|
||||
*,
|
||||
warn_on_unresolved: bool = True,
|
||||
_seen: set[int] | None = None,
|
||||
_expanded_paths: set[Path] | None = None,
|
||||
_keepalive: list[Any] | None = None,
|
||||
) -> None:
|
||||
obj: Any, *, warn_on_unresolved: bool = True
|
||||
) -> ForceLoadResult:
|
||||
"""Recursively resolve any deferred ``IncludeFile`` instances in a YAML tree.
|
||||
|
||||
Nested ``!include`` returns a deferred ``IncludeFile`` that is only resolved
|
||||
@@ -482,6 +493,23 @@ def force_load_include_files(
|
||||
run on a fresh re-parse where substitutions haven't been applied yet) to
|
||||
demote it to a debug log.
|
||||
"""
|
||||
result = ForceLoadResult([], [])
|
||||
_force_load_include_files(
|
||||
obj, warn_on_unresolved=warn_on_unresolved, _result=result
|
||||
)
|
||||
return result
|
||||
|
||||
|
||||
def _force_load_include_files(
|
||||
obj: Any,
|
||||
*,
|
||||
warn_on_unresolved: bool = True,
|
||||
_seen: set[int] | None = None,
|
||||
_expanded_paths: set[Path] | None = None,
|
||||
_keepalive: list[Any] | None = None,
|
||||
_result: ForceLoadResult,
|
||||
) -> None:
|
||||
"""Walk for :func:`force_load_include_files`, adding to ``result``."""
|
||||
from voluptuous import Invalid
|
||||
|
||||
if _seen is None:
|
||||
@@ -507,6 +535,7 @@ def force_load_include_files(
|
||||
seen=_seen,
|
||||
expanded_paths=_expanded_paths,
|
||||
keepalive=_keepalive,
|
||||
result=_result,
|
||||
)
|
||||
return
|
||||
try:
|
||||
@@ -518,37 +547,41 @@ def force_load_include_files(
|
||||
obj.parent_file,
|
||||
err,
|
||||
)
|
||||
_result.errors.append(f"{obj.file}: {err}")
|
||||
return
|
||||
force_load_include_files(
|
||||
_force_load_include_files(
|
||||
loaded,
|
||||
warn_on_unresolved=warn_on_unresolved,
|
||||
_seen=_seen,
|
||||
_expanded_paths=_expanded_paths,
|
||||
_keepalive=_keepalive,
|
||||
_result=_result,
|
||||
)
|
||||
elif isinstance(obj, dict):
|
||||
if id(obj) in _seen:
|
||||
return
|
||||
_seen.add(id(obj))
|
||||
for value in obj.values():
|
||||
force_load_include_files(
|
||||
_force_load_include_files(
|
||||
value,
|
||||
warn_on_unresolved=warn_on_unresolved,
|
||||
_seen=_seen,
|
||||
_expanded_paths=_expanded_paths,
|
||||
_keepalive=_keepalive,
|
||||
_result=_result,
|
||||
)
|
||||
elif isinstance(obj, (list, tuple)):
|
||||
if id(obj) in _seen:
|
||||
return
|
||||
_seen.add(id(obj))
|
||||
for item in obj:
|
||||
force_load_include_files(
|
||||
_force_load_include_files(
|
||||
item,
|
||||
warn_on_unresolved=warn_on_unresolved,
|
||||
_seen=_seen,
|
||||
_expanded_paths=_expanded_paths,
|
||||
_keepalive=_keepalive,
|
||||
_result=_result,
|
||||
)
|
||||
|
||||
|
||||
@@ -560,11 +593,16 @@ class DiscoveredYamlFiles:
|
||||
were re-parsing the user's config; ``secrets`` is the subset whose
|
||||
*un-resolved* filename matched :data:`esphome.const.SECRETS_FILES` (so
|
||||
a ``secrets.yaml`` symlinked to a differently-named target is still
|
||||
flagged as secrets).
|
||||
flagged as secrets). ``unresolved`` lists ``!include`` path strings that
|
||||
contain substitution variables and therefore could not be loaded, and
|
||||
``load_errors`` lists files that failed to parse or load — consumers
|
||||
should treat ``files`` as incomplete when either is non-empty.
|
||||
"""
|
||||
|
||||
files: list[Path] = field(default_factory=list)
|
||||
secrets: set[Path] = field(default_factory=set)
|
||||
unresolved: list[str] = field(default_factory=list)
|
||||
load_errors: list[str] = field(default_factory=list)
|
||||
|
||||
|
||||
def discover_user_yaml_files(config_path: Path) -> DiscoveredYamlFiles:
|
||||
@@ -594,9 +632,13 @@ def discover_user_yaml_files(config_path: Path) -> DiscoveredYamlFiles:
|
||||
try:
|
||||
try:
|
||||
data = load_yaml(config_path)
|
||||
except EsphomeError:
|
||||
return DiscoveredYamlFiles(list(loaded), secrets)
|
||||
force_load_include_files(data, warn_on_unresolved=False)
|
||||
except EsphomeError as err:
|
||||
return DiscoveredYamlFiles(
|
||||
list(loaded), secrets, load_errors=[f"{config_path}: {err}"]
|
||||
)
|
||||
unresolved, load_errors = force_load_include_files(
|
||||
data, warn_on_unresolved=False
|
||||
)
|
||||
finally:
|
||||
_load_listeners.remove(_capture_secret)
|
||||
|
||||
@@ -607,7 +649,7 @@ def discover_user_yaml_files(config_path: Path) -> DiscoveredYamlFiles:
|
||||
if path not in seen:
|
||||
seen.add(path)
|
||||
unique.append(path)
|
||||
return DiscoveredYamlFiles(unique, secrets)
|
||||
return DiscoveredYamlFiles(unique, secrets, unresolved, load_errors)
|
||||
|
||||
|
||||
def _add_data_ref(fn):
|
||||
|
||||
@@ -10,9 +10,11 @@ import pytest
|
||||
from esphome import bundle
|
||||
from esphome.components.packages import (
|
||||
CONFIG_SCHEMA,
|
||||
RemotePackageSource,
|
||||
_substitute_package_definition,
|
||||
_walk_packages,
|
||||
do_packages_pass,
|
||||
get_remote_package_sources,
|
||||
merge_packages,
|
||||
resolve_packages,
|
||||
)
|
||||
@@ -614,6 +616,49 @@ def test_package_remove_by_missing_id() -> None:
|
||||
assert actual == expected
|
||||
|
||||
|
||||
def test_get_remote_package_sources_empty_without_remote_packages() -> None:
|
||||
"""A config without remote packages records no sources."""
|
||||
assert get_remote_package_sources() == ()
|
||||
|
||||
|
||||
@patch("esphome.yaml_util.load_yaml")
|
||||
@patch("pathlib.Path.is_file")
|
||||
@patch("esphome.git.clone_or_update")
|
||||
def test_get_remote_package_sources_records_each_fetch_once(
|
||||
mock_clone_or_update, mock_is_file, mock_load_yaml
|
||||
) -> None:
|
||||
"""Every fetched remote source is recorded exactly once."""
|
||||
mock_clone_or_update.return_value = (Path("/tmp/noexists"), MagicMock())
|
||||
mock_is_file.return_value = True
|
||||
mock_load_yaml.side_effect = lambda *args, **kwargs: OrderedDict()
|
||||
|
||||
config = {
|
||||
CONF_PACKAGES: {
|
||||
"first": {
|
||||
CONF_URL: "https://github.com/esphome/repo-a",
|
||||
CONF_REF: "main",
|
||||
CONF_FILES: [TEST_YAML_FILENAME],
|
||||
},
|
||||
"again": {
|
||||
CONF_URL: "https://github.com/esphome/repo-a",
|
||||
CONF_REF: "main",
|
||||
CONF_FILES: ["other.yaml"],
|
||||
},
|
||||
"second": {
|
||||
CONF_URL: "https://github.com/esphome/repo-b",
|
||||
CONF_FILES: [TEST_YAML_FILENAME],
|
||||
},
|
||||
}
|
||||
}
|
||||
packages_pass(config)
|
||||
|
||||
sources = get_remote_package_sources()
|
||||
assert sorted(sources, key=lambda source: source.url) == [
|
||||
RemotePackageSource("https://github.com/esphome/repo-a", "main"),
|
||||
RemotePackageSource("https://github.com/esphome/repo-b", None),
|
||||
]
|
||||
|
||||
|
||||
@patch("esphome.yaml_util.load_yaml")
|
||||
@patch("pathlib.Path.is_file")
|
||||
@patch("esphome.git.clone_or_update")
|
||||
|
||||
@@ -1279,22 +1279,38 @@ def test_force_load_include_files_unresolved_log_level(
|
||||
assert matching == [expect_level]
|
||||
|
||||
|
||||
def test_force_load_include_files_returns_unresolved_paths(
|
||||
patch_include_file: None,
|
||||
) -> None:
|
||||
"""Includes with substitution-templated paths are reported back to the
|
||||
caller; resolvable ones are not."""
|
||||
templated = _StubInclude("${var}.yaml", unresolved=True)
|
||||
plain = _StubInclude("ok.yaml")
|
||||
result = force_load_include_files({"a": templated, "b": plain})
|
||||
assert result.unresolved == [str(templated.file)]
|
||||
assert result.errors == []
|
||||
assert plain.load_calls == 1
|
||||
|
||||
|
||||
def test_force_load_include_files_warns_on_load_failure(
|
||||
patch_include_file: None,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
"""An `EsphomeError` raised by `load()` is caught and logged, not propagated."""
|
||||
"""An `EsphomeError` raised by `load()` is caught, logged, and reported to
|
||||
the caller — not propagated."""
|
||||
stub = _StubInclude("missing.yaml", raise_on_load=EsphomeError("boom"))
|
||||
with caplog.at_level("WARNING", logger="esphome.yaml_util"):
|
||||
force_load_include_files({"k": stub})
|
||||
result = force_load_include_files({"k": stub})
|
||||
assert any(
|
||||
"Failed to load !include" in r.message and "missing.yaml" in r.message
|
||||
for r in caplog.records
|
||||
)
|
||||
assert result.errors == [f"{stub.file}: boom"]
|
||||
assert result.unresolved == []
|
||||
|
||||
|
||||
def test_discovered_yaml_files_holds_files_and_secrets() -> None:
|
||||
"""`DiscoveredYamlFiles` is a small data carrier; both fields are mandatory."""
|
||||
"""`DiscoveredYamlFiles` is a small data carrier."""
|
||||
files = [Path("/tmp/a.yaml")]
|
||||
secrets = {Path("/tmp/a.yaml")}
|
||||
discovered = DiscoveredYamlFiles(files, secrets)
|
||||
@@ -1357,11 +1373,24 @@ def test_discover_user_yaml_files_flags_secrets_symlink(tmp_path: Path) -> None:
|
||||
assert target.resolve() in discovered.secrets
|
||||
|
||||
|
||||
def test_discover_user_yaml_files_swallows_parse_errors(tmp_path: Path) -> None:
|
||||
"""A YAML parse failure returns whatever was tracked so far without raising."""
|
||||
def test_discover_user_yaml_files_reports_parse_errors(tmp_path: Path) -> None:
|
||||
"""A YAML parse failure is surfaced in `.load_errors` (not raised), so
|
||||
consumers can tell the file set is incomplete."""
|
||||
entry = _write(tmp_path, "entry.yaml", "esphome: [unterminated\n")
|
||||
discovered = discover_user_yaml_files(entry)
|
||||
assert isinstance(discovered, DiscoveredYamlFiles)
|
||||
assert len(discovered.load_errors) == 1
|
||||
assert "entry.yaml" in discovered.load_errors[0]
|
||||
|
||||
|
||||
def test_discover_user_yaml_files_reports_unresolved_includes(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""A substitution-templated `!include` path is surfaced in `.unresolved`."""
|
||||
entry = _write_entry_including(tmp_path, "${board}.yaml")
|
||||
discovered = discover_user_yaml_files(entry)
|
||||
assert len(discovered.unresolved) == 1
|
||||
assert "${board}.yaml" in discovered.unresolved[0]
|
||||
|
||||
|
||||
def test_discover_user_yaml_files_deduplicates(tmp_path: Path) -> None:
|
||||
|
||||
Reference in New Issue
Block a user