Add option to include vars in remote packages (#7606)

Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com>

esphome/components/packages/__init__.py

@@ -1,8 +1,8 @@
 from pathlib import Path
 
-import esphome.config_validation as cv
 from esphome import git, yaml_util
 from esphome.config_helpers import merge_config
+import esphome.config_validation as cv
 from esphome.const import (
     CONF_ESPHOME,
     CONF_FILE,
@@ -10,12 +10,14 @@ from esphome.const import (
     CONF_MIN_VERSION,
     CONF_PACKAGES,
     CONF_PASSWORD,
+    CONF_PATH,
     CONF_REF,
     CONF_REFRESH,
     CONF_URL,
     CONF_USERNAME,
+    CONF_VARS,
+    __version__ as ESPHOME_VERSION,
 )
-from esphome.const import __version__ as ESPHOME_VERSION
 from esphome.core import EsphomeError
 
 DOMAIN = CONF_PACKAGES
@@ -74,7 +76,19 @@ BASE_SCHEMA = cv.All(
             cv.Optional(CONF_PASSWORD): cv.string,
             cv.Exclusive(CONF_FILE, "files"): validate_yaml_filename,
             cv.Exclusive(CONF_FILES, "files"): cv.All(
-                cv.ensure_list(validate_yaml_filename),
+                cv.ensure_list(
+                    cv.Any(
+                        validate_yaml_filename,
+                        cv.Schema(
+                            {
+                                cv.Required(CONF_PATH): validate_yaml_filename,
+                                cv.Optional(CONF_VARS, default={}): cv.Schema(
+                                    {cv.string: cv.string}
+                                ),
+                            }
+                        ),
+                    )
+                ),
                 cv.Length(min=1),
             ),
             cv.Optional(CONF_REF): cv.git_ref,
@@ -106,16 +120,25 @@ def _process_base_package(config: dict) -> dict:
         username=config.get(CONF_USERNAME),
         password=config.get(CONF_PASSWORD),
     )
-    files: list[str] = config[CONF_FILES]
+    files = []
+
+    for file in config[CONF_FILES]:
+        if isinstance(file, str):
+            files.append({CONF_PATH: file, CONF_VARS: {}})
+        else:
+            files.append(file)
 
     def get_packages(files) -> dict:
         packages = {}
-        for file in files:
+        for idx, file in enumerate(files):
-            yaml_file: Path = repo_dir / file
+            filename = file[CONF_PATH]
+            yaml_file: Path = repo_dir / filename
+            vars = file.get(CONF_VARS, {})
 
             if not yaml_file.is_file():
                 raise cv.Invalid(
-                    f"{file} does not exist in repository", path=[CONF_FILES]
+                    f"{filename} does not exist in repository",
+                    path=[CONF_FILES, idx, CONF_PATH],
                 )
 
             try:
@@ -131,11 +154,12 @@ def _process_base_package(config: dict) -> dict:
                         raise cv.Invalid(
                             f"Current ESPHome Version is too old to use this package: {ESPHOME_VERSION} < {min_version}"
                         )
-
+                vars = {k: str(v) for k, v in vars.items()}
-                packages[file] = new_yaml
+                new_yaml = yaml_util.substitute_vars(new_yaml, vars)
+                packages[f"{filename}{idx}"] = new_yaml
             except EsphomeError as e:
                 raise cv.Invalid(
-                    f"{file} is not a valid YAML file. Please check the file contents.\n{e}"
+                    f"{filename} is not a valid YAML file. Please check the file contents.\n{e}"
                 ) from e
         return packages
 
@@ -154,7 +178,7 @@ def _process_base_package(config: dict) -> dict:
             error = er
 
     if packages is None:
-        raise cv.Invalid(f"Failed to load packages. {error}")
+        raise cv.Invalid(f"Failed to load packages. {error}", path=error.path)
 
     return {"packages": packages}
 

esphome/const.py

@@ -927,6 +927,7 @@ CONF_VALUE = "value"
 CONF_VALUE_FONT = "value_font"
 CONF_VARIABLES = "variables"
 CONF_VARIANT = "variant"
+CONF_VARS = "vars"
 CONF_VERSION = "version"
 CONF_VIBRATIONS = "vibrations"
 CONF_VISIBLE = "visible"

esphome/yaml_util.py

@@ -273,48 +273,18 @@ class ESPHomeLoaderMixin:
 
     @_add_data_ref
     def construct_include(self, node):
+        from esphome.const import CONF_VARS
+
         def extract_file_vars(node):
             fields = self.construct_yaml_map(node)
             file = fields.get("file")
             if file is None:
                 raise yaml.MarkedYAMLError("Must include 'file'", node.start_mark)
-            vars = fields.get("vars")
+            vars = fields.get(CONF_VARS)
             if vars:
                 vars = {k: str(v) for k, v in vars.items()}
             return file, vars
 
-        def substitute_vars(config, vars):
-            from esphome.components import substitutions
-            from esphome.const import CONF_DEFAULTS, CONF_SUBSTITUTIONS
-
-            org_subs = None
-            result = config
-            if not isinstance(config, dict):
-                # when the included yaml contains a list or a scalar
-                # wrap it into an OrderedDict because do_substitution_pass expects it
-                result = OrderedDict([("yaml", config)])
-            elif CONF_SUBSTITUTIONS in result:
-                org_subs = result.pop(CONF_SUBSTITUTIONS)
-
-            defaults = {}
-            if CONF_DEFAULTS in result:
-                defaults = result.pop(CONF_DEFAULTS)
-
-            result[CONF_SUBSTITUTIONS] = vars
-            for k, v in defaults.items():
-                if k not in result[CONF_SUBSTITUTIONS]:
-                    result[CONF_SUBSTITUTIONS][k] = v
-
-            # Ignore missing vars that refer to the top level substitutions
-            substitutions.do_substitution_pass(result, None, ignore_missing=True)
-            result.pop(CONF_SUBSTITUTIONS)
-
-            if not isinstance(config, dict):
-                result = result["yaml"]  # unwrap the result
-            elif org_subs:
-                result[CONF_SUBSTITUTIONS] = org_subs
-            return result
-
         if isinstance(node, yaml.nodes.MappingNode):
             file, vars = extract_file_vars(node)
         else:
@@ -432,6 +402,39 @@ def parse_yaml(file_name: str, file_handle: TextIOWrapper) -> Any:
         )
 
 
+def substitute_vars(config, vars):
+    from esphome.components import substitutions
+    from esphome.const import CONF_DEFAULTS, CONF_SUBSTITUTIONS
+
+    org_subs = None
+    result = config
+    if not isinstance(config, dict):
+        # when the included yaml contains a list or a scalar
+        # wrap it into an OrderedDict because do_substitution_pass expects it
+        result = OrderedDict([("yaml", config)])
+    elif CONF_SUBSTITUTIONS in result:
+        org_subs = result.pop(CONF_SUBSTITUTIONS)
+
+    defaults = {}
+    if CONF_DEFAULTS in result:
+        defaults = result.pop(CONF_DEFAULTS)
+
+    result[CONF_SUBSTITUTIONS] = vars
+    for k, v in defaults.items():
+        if k not in result[CONF_SUBSTITUTIONS]:
+            result[CONF_SUBSTITUTIONS][k] = v
+
+    # Ignore missing vars that refer to the top level substitutions
+    substitutions.do_substitution_pass(result, None, ignore_missing=True)
+    result.pop(CONF_SUBSTITUTIONS)
+
+    if not isinstance(config, dict):
+        result = result["yaml"]  # unwrap the result
+    elif org_subs:
+        result[CONF_SUBSTITUTIONS] = org_subs
+    return result
+
+
 def _load_yaml_internal(fname: str) -> Any:
     """Load a YAML file."""
     try:

tests/component_tests/packages/test_packages.py

@@ -1,11 +1,18 @@
 """Tests for the packages component."""
 
+from pathlib import Path
+from unittest.mock import MagicMock, patch
+
 import pytest
 
-
+from esphome.components.packages import do_packages_pass
+from esphome.config_helpers import Extend, Remove
+import esphome.config_validation as cv
 from esphome.const import (
+    CONF_DEFAULTS,
     CONF_DOMAIN,
     CONF_ESPHOME,
+    CONF_FILES,
     CONF_FILTERS,
     CONF_ID,
     CONF_MULTIPLY,
@@ -13,15 +20,18 @@ from esphome.const import (
     CONF_OFFSET,
     CONF_PACKAGES,
     CONF_PASSWORD,
+    CONF_PATH,
     CONF_PLATFORM,
+    CONF_REF,
+    CONF_REFRESH,
     CONF_SENSOR,
     CONF_SSID,
     CONF_UPDATE_INTERVAL,
+    CONF_URL,
+    CONF_VARS,
     CONF_WIFI,
 )
-from esphome.components.packages import do_packages_pass
+from esphome.util import OrderedDict
-from esphome.config_helpers import Extend, Remove
-import esphome.config_validation as cv
 
 # Test strings
 TEST_DEVICE_NAME = "test_device_name"
@@ -34,9 +44,11 @@ TEST_SENSOR_PLATFORM_1 = "test_sensor_platform_1"
 TEST_SENSOR_PLATFORM_2 = "test_sensor_platform_2"
 TEST_SENSOR_NAME_1 = "test_sensor_name_1"
 TEST_SENSOR_NAME_2 = "test_sensor_name_2"
+TEST_SENSOR_NAME_3 = "test_sensor_name_3"
 TEST_SENSOR_ID_1 = "test_sensor_id_1"
 TEST_SENSOR_ID_2 = "test_sensor_id_2"
 TEST_SENSOR_UPDATE_INTERVAL = "test_sensor_update_interval"
+TEST_YAML_FILENAME = "sensor1.yaml"
 
 
 @pytest.fixture(name="basic_wifi")
@@ -188,17 +200,35 @@ def test_package_list_merge():
             }
         },
         CONF_SENSOR: [
-            {CONF_PLATFORM: TEST_SENSOR_PLATFORM_2, CONF_NAME: TEST_SENSOR_NAME_1},
+            {
-            {CONF_PLATFORM: TEST_SENSOR_PLATFORM_2, CONF_NAME: TEST_SENSOR_NAME_2},
+                CONF_PLATFORM: TEST_SENSOR_PLATFORM_2,
+                CONF_NAME: TEST_SENSOR_NAME_1,
+            },
+            {
+                CONF_PLATFORM: TEST_SENSOR_PLATFORM_2,
+                CONF_NAME: TEST_SENSOR_NAME_2,
+            },
         ],
     }
 
     expected = {
         CONF_SENSOR: [
-            {CONF_PLATFORM: TEST_SENSOR_PLATFORM_1, CONF_NAME: TEST_SENSOR_NAME_1},
+            {
-            {CONF_PLATFORM: TEST_SENSOR_PLATFORM_1, CONF_NAME: TEST_SENSOR_NAME_2},
+                CONF_PLATFORM: TEST_SENSOR_PLATFORM_1,
-            {CONF_PLATFORM: TEST_SENSOR_PLATFORM_2, CONF_NAME: TEST_SENSOR_NAME_1},
+                CONF_NAME: TEST_SENSOR_NAME_1,
-            {CONF_PLATFORM: TEST_SENSOR_PLATFORM_2, CONF_NAME: TEST_SENSOR_NAME_2},
+            },
+            {
+                CONF_PLATFORM: TEST_SENSOR_PLATFORM_1,
+                CONF_NAME: TEST_SENSOR_NAME_2,
+            },
+            {
+                CONF_PLATFORM: TEST_SENSOR_PLATFORM_2,
+                CONF_NAME: TEST_SENSOR_NAME_1,
+            },
+            {
+                CONF_PLATFORM: TEST_SENSOR_PLATFORM_2,
+                CONF_NAME: TEST_SENSOR_NAME_2,
+            },
         ]
     }
 
@@ -252,7 +282,10 @@ def test_package_list_merge_by_id():
                 CONF_UPDATE_INTERVAL: TEST_SENSOR_UPDATE_INTERVAL,
             },
             {CONF_ID: Extend(TEST_SENSOR_ID_2), CONF_NAME: TEST_SENSOR_NAME_1},
-            {CONF_PLATFORM: TEST_SENSOR_PLATFORM_2, CONF_NAME: TEST_SENSOR_NAME_2},
+            {
+                CONF_PLATFORM: TEST_SENSOR_PLATFORM_2,
+                CONF_NAME: TEST_SENSOR_NAME_2,
+            },
         ],
     }
 
@@ -270,7 +303,10 @@ def test_package_list_merge_by_id():
                 CONF_PLATFORM: TEST_SENSOR_PLATFORM_1,
                 CONF_NAME: TEST_SENSOR_NAME_1,
             },
-            {CONF_PLATFORM: TEST_SENSOR_PLATFORM_2, CONF_NAME: TEST_SENSOR_NAME_2},
+            {
+                CONF_PLATFORM: TEST_SENSOR_PLATFORM_2,
+                CONF_NAME: TEST_SENSOR_NAME_2,
+            },
         ]
     }
 
@@ -289,12 +325,18 @@ def test_package_merge_by_id_with_list():
         CONF_PACKAGES: {
             "sensors": {
                 CONF_SENSOR: [
-                    {CONF_ID: TEST_SENSOR_ID_1, CONF_FILTERS: [{CONF_MULTIPLY: 42.0}]}
+                    {
+                        CONF_ID: TEST_SENSOR_ID_1,
+                        CONF_FILTERS: [{CONF_MULTIPLY: 42.0}],
+                    }
                 ]
             }
         },
         CONF_SENSOR: [
-            {CONF_ID: Extend(TEST_SENSOR_ID_1), CONF_FILTERS: [{CONF_OFFSET: 146.0}]}
+            {
+                CONF_ID: Extend(TEST_SENSOR_ID_1),
+                CONF_FILTERS: [{CONF_OFFSET: 146.0}],
+            }
         ],
     }
 
@@ -320,13 +362,19 @@ def test_package_merge_by_missing_id():
         CONF_PACKAGES: {
             "sensors": {
                 CONF_SENSOR: [
-                    {CONF_ID: TEST_SENSOR_ID_1, CONF_FILTERS: [{CONF_MULTIPLY: 42.0}]},
+                    {
+                        CONF_ID: TEST_SENSOR_ID_1,
+                        CONF_FILTERS: [{CONF_MULTIPLY: 42.0}],
+                    },
                 ]
             }
         },
         CONF_SENSOR: [
             {CONF_ID: TEST_SENSOR_ID_1, CONF_FILTERS: [{CONF_MULTIPLY: 10.0}]},
-            {CONF_ID: Extend(TEST_SENSOR_ID_2), CONF_FILTERS: [{CONF_OFFSET: 146.0}]},
+            {
+                CONF_ID: Extend(TEST_SENSOR_ID_2),
+                CONF_FILTERS: [{CONF_OFFSET: 146.0}],
+            },
         ],
     }
 
@@ -451,8 +499,6 @@ def test_multiple_package_list_remove_by_id():
 def test_package_dict_remove_by_id(basic_wifi, basic_esphome):
     """
     Ensures that components with missing IDs are removed from dict.
-    """
-    """
     Ensures that the top-level configuration takes precedence over duplicate keys defined in a package.
 
     In this test, CONF_SSID should be overwritten by that defined in the top-level config.
@@ -480,14 +526,20 @@ def test_package_remove_by_missing_id():
         CONF_PACKAGES: {
             "sensors": {
                 CONF_SENSOR: [
-                    {CONF_ID: TEST_SENSOR_ID_1, CONF_FILTERS: [{CONF_MULTIPLY: 42.0}]},
+                    {
+                        CONF_ID: TEST_SENSOR_ID_1,
+                        CONF_FILTERS: [{CONF_MULTIPLY: 42.0}],
+                    },
                 ]
             }
         },
         "missing_key": Remove(),
         CONF_SENSOR: [
             {CONF_ID: TEST_SENSOR_ID_1, CONF_FILTERS: [{CONF_MULTIPLY: 10.0}]},
-            {CONF_ID: Remove(TEST_SENSOR_ID_2), CONF_FILTERS: [{CONF_OFFSET: 146.0}]},
+            {
+                CONF_ID: Remove(TEST_SENSOR_ID_2),
+                CONF_FILTERS: [{CONF_OFFSET: 146.0}],
+            },
         ],
     }
 
@@ -511,3 +563,171 @@ def test_package_remove_by_missing_id():
 
     actual = do_packages_pass(config)
     assert actual == expected
+
+
+@patch("esphome.yaml_util.load_yaml")
+@patch("pathlib.Path.is_file")
+@patch("esphome.git.clone_or_update")
+def test_remote_packages_with_files_list(
+    mock_clone_or_update, mock_is_file, mock_load_yaml
+):
+    """
+    Ensures that packages are loaded as mixed list of dictionary and strings
+    """
+    # Mock the response from git.clone_or_update
+    mock_revert = MagicMock()
+    mock_clone_or_update.return_value = (Path("/tmp/noexists"), mock_revert)
+
+    # Mock the response from pathlib.Path.is_file
+    mock_is_file.return_value = True
+
+    # Mock the response from esphome.yaml_util.load_yaml
+    mock_load_yaml.side_effect = [
+        OrderedDict(
+            {
+                CONF_SENSOR: [
+                    {
+                        CONF_PLATFORM: TEST_SENSOR_PLATFORM_1,
+                        CONF_NAME: TEST_SENSOR_NAME_1,
+                    }
+                ]
+            }
+        ),
+        OrderedDict(
+            {
+                CONF_SENSOR: [
+                    {
+                        CONF_PLATFORM: TEST_SENSOR_PLATFORM_1,
+                        CONF_NAME: TEST_SENSOR_NAME_2,
+                    }
+                ]
+            }
+        ),
+    ]
+
+    # Define the input config
+    config = {
+        CONF_PACKAGES: {
+            "package1": {
+                CONF_URL: "https://github.com/esphome/non-existant-repo",
+                CONF_REF: "main",
+                CONF_FILES: [
+                    {CONF_PATH: TEST_YAML_FILENAME},
+                    "sensor2.yaml",
+                ],
+                CONF_REFRESH: "1d",
+            }
+        }
+    }
+
+    expected = {
+        CONF_SENSOR: [
+            {
+                CONF_PLATFORM: TEST_SENSOR_PLATFORM_1,
+                CONF_NAME: TEST_SENSOR_NAME_1,
+            },
+            {
+                CONF_PLATFORM: TEST_SENSOR_PLATFORM_1,
+                CONF_NAME: TEST_SENSOR_NAME_2,
+            },
+        ]
+    }
+
+    actual = do_packages_pass(config)
+    assert actual == expected
+
+
+@patch("esphome.yaml_util.load_yaml")
+@patch("pathlib.Path.is_file")
+@patch("esphome.git.clone_or_update")
+def test_remote_packages_with_files_and_vars(
+    mock_clone_or_update, mock_is_file, mock_load_yaml
+):
+    """
+    Ensures that packages are loaded as mixed list of dictionary and strings with vars
+    """
+    # Mock the response from git.clone_or_update
+    mock_revert = MagicMock()
+    mock_clone_or_update.return_value = (Path("/tmp/noexists"), mock_revert)
+
+    # Mock the response from pathlib.Path.is_file
+    mock_is_file.return_value = True
+
+    # Mock the response from esphome.yaml_util.load_yaml
+    mock_load_yaml.side_effect = [
+        OrderedDict(
+            {
+                CONF_DEFAULTS: {CONF_NAME: TEST_SENSOR_NAME_1},
+                CONF_SENSOR: [
+                    {
+                        CONF_PLATFORM: TEST_SENSOR_PLATFORM_1,
+                        CONF_NAME: "${name}",
+                    }
+                ],
+            }
+        ),
+        OrderedDict(
+            {
+                CONF_DEFAULTS: {CONF_NAME: TEST_SENSOR_NAME_1},
+                CONF_SENSOR: [
+                    {
+                        CONF_PLATFORM: TEST_SENSOR_PLATFORM_1,
+                        CONF_NAME: "${name}",
+                    }
+                ],
+            }
+        ),
+        OrderedDict(
+            {
+                CONF_DEFAULTS: {CONF_NAME: TEST_SENSOR_NAME_1},
+                CONF_SENSOR: [
+                    {
+                        CONF_PLATFORM: TEST_SENSOR_PLATFORM_1,
+                        CONF_NAME: "${name}",
+                    }
+                ],
+            }
+        ),
+    ]
+
+    # Define the input config
+    config = {
+        CONF_PACKAGES: {
+            "package1": {
+                CONF_URL: "https://github.com/esphome/non-existant-repo",
+                CONF_REF: "main",
+                CONF_FILES: [
+                    {
+                        CONF_PATH: TEST_YAML_FILENAME,
+                        CONF_VARS: {CONF_NAME: TEST_SENSOR_NAME_2},
+                    },
+                    {
+                        CONF_PATH: TEST_YAML_FILENAME,
+                        CONF_VARS: {CONF_NAME: TEST_SENSOR_NAME_3},
+                    },
+                    {CONF_PATH: TEST_YAML_FILENAME},
+                ],
+                CONF_REFRESH: "1d",
+            }
+        }
+    }
+
+    expected = {
+        CONF_SENSOR: [
+            {
+                CONF_PLATFORM: TEST_SENSOR_PLATFORM_1,
+                CONF_NAME: TEST_SENSOR_NAME_2,
+            },
+            {
+                CONF_PLATFORM: TEST_SENSOR_PLATFORM_1,
+                CONF_NAME: TEST_SENSOR_NAME_3,
+            },
+            {
+                CONF_PLATFORM: TEST_SENSOR_PLATFORM_1,
+                CONF_NAME: TEST_SENSOR_NAME_1,
+            },
+        ]
+    }
+
+    actual = do_packages_pass(config)
+    assert actual == expected