diff --git a/homeassistant/config.py b/homeassistant/config.py index 8a868018adf..3e593a564a2 100644 --- a/homeassistant/config.py +++ b/homeassistant/config.py @@ -212,9 +212,11 @@ def _filter_bad_internal_external_urls(conf: dict) -> dict: return conf -PACKAGES_CONFIG_SCHEMA = cv.schema_with_slug_keys( # Package names are slugs - vol.Schema({cv.string: vol.Any(dict, list, None)}) # Component config -) +# Schema for all packages element +PACKAGES_CONFIG_SCHEMA = vol.Schema({cv.string: vol.Any(dict, list)}) + +# Schema for individual package definition +PACKAGE_DEFINITION_SCHEMA = vol.Schema({cv.string: vol.Any(dict, list, None)}) CUSTOMIZE_DICT_SCHEMA = vol.Schema( { @@ -499,7 +501,17 @@ async def async_hass_config_yaml(hass: HomeAssistant) -> dict: config.pop(invalid_domain) core_config = config.get(CONF_CORE, {}) - await merge_packages_config(hass, config, core_config.get(CONF_PACKAGES, {})) + try: + await merge_packages_config(hass, config, core_config.get(CONF_PACKAGES, {})) + except vol.Invalid as exc: + suffix = "" + if annotation := find_annotation(config, [CONF_CORE, CONF_PACKAGES] + exc.path): + suffix = f" at {_relpath(hass, annotation[0])}, line {annotation[1]}" + _LOGGER.error( + "Invalid package configuration '%s'%s: %s", CONF_PACKAGES, suffix, exc + ) + core_config[CONF_PACKAGES] = {} + return config @@ -938,7 +950,7 @@ async def async_process_ha_core_config(hass: HomeAssistant, config: dict) -> Non def _log_pkg_error( - hass: HomeAssistant, package: str, component: str, config: dict, message: str + hass: HomeAssistant, package: str, component: str | None, config: dict, message: str ) -> None: """Log an error while merging packages.""" message_prefix = f"Setup of package '{package}'" @@ -996,6 +1008,12 @@ def _identify_config_schema(module: ComponentProtocol) -> str | None: return None +def _validate_package_definition(name: str, conf: Any) -> None: + """Validate basic package definition properties.""" + cv.slug(name) + PACKAGE_DEFINITION_SCHEMA(conf) + + def _recursive_merge(conf: dict[str, Any], package: dict[str, Any]) -> str | None: """Merge package into conf, recursively.""" duplicate_key: str | None = None @@ -1023,12 +1041,33 @@ async def merge_packages_config( config: dict, packages: dict[str, Any], _log_pkg_error: Callable[ - [HomeAssistant, str, str, dict, str], None + [HomeAssistant, str, str | None, dict, str], None ] = _log_pkg_error, ) -> dict: - """Merge packages into the top-level configuration. Mutate config.""" + """Merge packages into the top-level configuration. + + Ignores packages that cannot be setup. Mutates config. Raises + vol.Invalid if whole package config is invalid. + """ + PACKAGES_CONFIG_SCHEMA(packages) + + invalid_packages = [] for pack_name, pack_conf in packages.items(): + try: + _validate_package_definition(pack_name, pack_conf) + except vol.Invalid as exc: + _log_pkg_error( + hass, + pack_name, + None, + config, + f"Invalid package definition '{pack_name}': {str(exc)}. Package " + f"will not be initialized", + ) + invalid_packages.append(pack_name) + continue + for comp_name, comp_conf in pack_conf.items(): if comp_name == CONF_CORE: continue @@ -1123,6 +1162,9 @@ async def merge_packages_config( f"integration '{comp_name}' has duplicate key '{duplicate_key}'", ) + for pack_name in invalid_packages: + packages.pop(pack_name, {}) + return config diff --git a/homeassistant/helpers/check_config.py b/homeassistant/helpers/check_config.py index 1c8efadfdc5..b362d68ad55 100644 --- a/homeassistant/helpers/check_config.py +++ b/homeassistant/helpers/check_config.py @@ -96,13 +96,13 @@ async def async_check_ha_config_file( # noqa: C901 def _pack_error( hass: HomeAssistant, package: str, - component: str, + component: str | None, config: ConfigType, message: str, ) -> None: """Handle errors from packages.""" message = f"Setup of package '{package}' failed: {message}" - domain = f"homeassistant.packages.{package}.{component}" + domain = f"homeassistant.packages.{package}{'.' + component if component is not None else ''}" pack_config = core_config[CONF_PACKAGES].get(package, config) result.add_warning(message, domain, pack_config) @@ -157,10 +157,15 @@ async def async_check_ha_config_file( # noqa: C901 return result.add_error(f"Error loading {config_path}: {err}") # Extract and validate core [homeassistant] config + core_config = config.pop(CONF_CORE, {}) try: - core_config = config.pop(CONF_CORE, {}) core_config = CORE_CONFIG_SCHEMA(core_config) result[CONF_CORE] = core_config + + # Merge packages + await merge_packages_config( + hass, config, core_config.get(CONF_PACKAGES, {}), _pack_error + ) except vol.Invalid as err: result.add_error( format_schema_error(hass, err, CONF_CORE, core_config), @@ -168,11 +173,6 @@ async def async_check_ha_config_file( # noqa: C901 core_config, ) core_config = {} - - # Merge packages - await merge_packages_config( - hass, config, core_config.get(CONF_PACKAGES, {}), _pack_error - ) core_config.pop(CONF_PACKAGES, None) # Filter out repeating config sections diff --git a/tests/fixtures/core/config/package_schema_errors/packages_is_a_list/configuration.yaml b/tests/fixtures/core/config/package_schema_errors/packages_is_a_list/configuration.yaml new file mode 100644 index 00000000000..803864cda23 --- /dev/null +++ b/tests/fixtures/core/config/package_schema_errors/packages_is_a_list/configuration.yaml @@ -0,0 +1,5 @@ +homeassistant: + packages: + - should_be_a_dict_not_a_list + - this_also_fails: + iot_domain: [{ "some": "yay" }] diff --git a/tests/fixtures/core/config/package_schema_errors/packages_is_a_value/configuration.yaml b/tests/fixtures/core/config/package_schema_errors/packages_is_a_value/configuration.yaml new file mode 100644 index 00000000000..d9a471d7773 --- /dev/null +++ b/tests/fixtures/core/config/package_schema_errors/packages_is_a_value/configuration.yaml @@ -0,0 +1,2 @@ +homeassistant: + packages: "Hello" diff --git a/tests/fixtures/core/config/package_schema_errors/packages_is_null/configuration.yaml b/tests/fixtures/core/config/package_schema_errors/packages_is_null/configuration.yaml new file mode 100644 index 00000000000..9d9801f8eab --- /dev/null +++ b/tests/fixtures/core/config/package_schema_errors/packages_is_null/configuration.yaml @@ -0,0 +1,3 @@ +homeassistant: + name: Ensure Correct Line Numbers + packages: null diff --git a/tests/fixtures/core/config/package_schema_validation/packages_dict/configuration.yaml b/tests/fixtures/core/config/package_schema_validation/packages_dict/configuration.yaml new file mode 100644 index 00000000000..8ebed2b339b --- /dev/null +++ b/tests/fixtures/core/config/package_schema_validation/packages_dict/configuration.yaml @@ -0,0 +1,6 @@ +homeassistant: + packages: + should_be_a_dict: + - platform: template + this_works: + iot_domain: [{ "some": "yay" }] diff --git a/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_dict/configuration.yaml b/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_dict/configuration.yaml new file mode 100644 index 00000000000..00751771c68 --- /dev/null +++ b/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_dict/configuration.yaml @@ -0,0 +1,2 @@ +homeassistant: + packages: !include_dir_merge_named packages/ diff --git a/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_dict/packages/expected_dict.yaml b/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_dict/packages/expected_dict.yaml new file mode 100644 index 00000000000..6ac66367fd9 --- /dev/null +++ b/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_dict/packages/expected_dict.yaml @@ -0,0 +1,2 @@ +should_be_a_dict: + - platform: template diff --git a/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_dict/packages/this_works.yaml b/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_dict/packages/this_works.yaml new file mode 100644 index 00000000000..076e556d3d8 --- /dev/null +++ b/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_dict/packages/this_works.yaml @@ -0,0 +1,2 @@ +this_works: + iot_domain: [{ "some": "yay" }] diff --git a/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_slug/configuration.yaml b/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_slug/configuration.yaml new file mode 100644 index 00000000000..00751771c68 --- /dev/null +++ b/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_slug/configuration.yaml @@ -0,0 +1,2 @@ +homeassistant: + packages: !include_dir_merge_named packages/ diff --git a/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_slug/packages/expected_slug.yaml b/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_slug/packages/expected_slug.yaml new file mode 100644 index 00000000000..05bc492e986 --- /dev/null +++ b/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_slug/packages/expected_slug.yaml @@ -0,0 +1,3 @@ +this is not a slug but it should be one: + switch: + - platform: template diff --git a/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_slug/packages/this_works.yaml b/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_slug/packages/this_works.yaml new file mode 100644 index 00000000000..076e556d3d8 --- /dev/null +++ b/tests/fixtures/core/config/package_schema_validation/packages_include_dir_named_slug/packages/this_works.yaml @@ -0,0 +1,2 @@ +this_works: + iot_domain: [{ "some": "yay" }] diff --git a/tests/fixtures/core/config/package_schema_validation/packages_slug/configuration.yaml b/tests/fixtures/core/config/package_schema_validation/packages_slug/configuration.yaml new file mode 100644 index 00000000000..ee67338102e --- /dev/null +++ b/tests/fixtures/core/config/package_schema_validation/packages_slug/configuration.yaml @@ -0,0 +1,7 @@ +homeassistant: + packages: + this is not a slug but it should be one: + switch: + - platform: template + this_works: + iot_domain: [{ "some": "yay" }] diff --git a/tests/helpers/test_check_config.py b/tests/helpers/test_check_config.py index de57fa0a8f3..edf1066f744 100644 --- a/tests/helpers/test_check_config.py +++ b/tests/helpers/test_check_config.py @@ -374,7 +374,7 @@ async def test_platform_import_error(hass: HomeAssistant) -> None: async def test_package_invalid(hass: HomeAssistant) -> None: - """Test a valid platform setup.""" + """Test a platform setup with an invalid package config.""" files = {YAML_CONFIG_FILE: BASE_CONFIG + ' packages:\n p1:\n group: ["a"]'} with patch("os.path.isfile", return_value=True), patch_yaml_files(files): res = await async_check_ha_config_file(hass) @@ -393,6 +393,72 @@ async def test_package_invalid(hass: HomeAssistant) -> None: _assert_warnings_errors(res, [warning], []) +async def test_package_definition_invalid_slug_keys(hass: HomeAssistant) -> None: + """Test a platform setup with a broken package: keys must be slugs.""" + files = { + YAML_CONFIG_FILE: BASE_CONFIG + + ' packages:\n not a slug:\n group: ["a"]' + } + with patch("os.path.isfile", return_value=True), patch_yaml_files(files): + res = await async_check_ha_config_file(hass) + log_ha_config(res) + + assert res.keys() == {"homeassistant"} + + warning = CheckConfigError( + ( + "Setup of package 'not a slug' failed: Invalid package definition 'not a slug': invalid slug not a " + "slug (try not_a_slug). Package will not be initialized" + ), + "homeassistant.packages.not a slug", + {"group": ["a"]}, + ) + _assert_warnings_errors(res, [warning], []) + + +async def test_package_definition_invalid_dict(hass: HomeAssistant) -> None: + """Test a platform setup with a broken package: packages must be dicts.""" + files = { + YAML_CONFIG_FILE: BASE_CONFIG + + ' packages:\n not_a_dict:\n - group: ["a"]' + } + with patch("os.path.isfile", return_value=True), patch_yaml_files(files): + res = await async_check_ha_config_file(hass) + log_ha_config(res) + + assert res.keys() == {"homeassistant"} + + warning = CheckConfigError( + ( + "Setup of package 'not_a_dict' failed: Invalid package definition 'not_a_dict': expected a " + "dictionary. Package will not be initialized" + ), + "homeassistant.packages.not_a_dict", + [{"group": ["a"]}], + ) + _assert_warnings_errors(res, [warning], []) + + +async def test_package_schema_invalid(hass: HomeAssistant) -> None: + """Test an invalid platform config because of severely broken packages section.""" + files = { + YAML_CONFIG_FILE: "homeassistant:\n packages:\n - must\n - not\n - be\n - a\n - list" + } + with patch("os.path.isfile", return_value=True), patch_yaml_files(files): + res = await async_check_ha_config_file(hass) + log_ha_config(res) + + error = CheckConfigError( + ( + f"Invalid config for 'homeassistant' at {YAML_CONFIG_FILE}, line 2:" + " expected a dictionary for dictionary value 'packages', got ['must', 'not', 'be', 'a', 'list']" + ), + "homeassistant", + {"packages": ["must", "not", "be", "a", "list"]}, + ) + _assert_warnings_errors(res, [], [error]) + + async def test_missing_included_file(hass: HomeAssistant) -> None: """Test missing included file.""" files = {YAML_CONFIG_FILE: BASE_CONFIG + "automation: !include no.yaml"} diff --git a/tests/snapshots/test_config.ambr b/tests/snapshots/test_config.ambr index 76d3f0c4666..34fa5d61797 100644 --- a/tests/snapshots/test_config.ambr +++ b/tests/snapshots/test_config.ambr @@ -451,3 +451,38 @@ ''', ]) # --- +# name: test_individual_packages_schema_validation_errors[packages_dict] + list([ + "Setup of package 'should_be_a_dict' at configuration.yaml, line 3 failed: Invalid package definition 'should_be_a_dict': expected a dictionary. Package will not be initialized", + ]) +# --- +# name: test_individual_packages_schema_validation_errors[packages_slug] + list([ + "Setup of package 'this is not a slug but it should be one' at configuration.yaml, line 3 failed: Invalid package definition 'this is not a slug but it should be one': invalid slug this is not a slug but it should be one (try this_is_not_a_slug_but_it_should_be_one). Package will not be initialized", + ]) +# --- +# name: test_individual_packages_schema_validation_errors[packages_include_dir_named_dict] + list([ + "Setup of package 'should_be_a_dict' at packages/expected_dict.yaml, line 1 failed: Invalid package definition 'should_be_a_dict': expected a dictionary. Package will not be initialized", + ]) +# --- +# name: test_individual_packages_schema_validation_errors[packages_include_dir_named_slug] + list([ + "Setup of package 'this is not a slug but it should be one' at packages/expected_slug.yaml, line 1 failed: Invalid package definition 'this is not a slug but it should be one': invalid slug this is not a slug but it should be one (try this_is_not_a_slug_but_it_should_be_one). Package will not be initialized", + ]) +# --- +# name: test_packages_schema_validation_error[packages_is_a_list] + list([ + "Invalid package configuration 'packages' at configuration.yaml, line 2: expected a dictionary", + ]) +# --- +# name: test_packages_schema_validation_error[packages_is_a_value] + list([ + "Invalid package configuration 'packages' at configuration.yaml, line 2: expected a dictionary", + ]) +# --- +# name: test_packages_schema_validation_error[packages_is_null] + list([ + "Invalid package configuration 'packages' at configuration.yaml, line 3: expected a dictionary", + ]) +# --- diff --git a/tests/test_config.py b/tests/test_config.py index 0f6b36d90b5..36d4351afb4 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -2233,6 +2233,79 @@ async def test_yaml_error( assert error_records == snapshot +@pytest.mark.parametrize( + "config_dir", + [ + "packages_dict", + "packages_slug", + "packages_include_dir_named_dict", + "packages_include_dir_named_slug", + ], +) +async def test_individual_packages_schema_validation_errors( + hass: HomeAssistant, + caplog: pytest.LogCaptureFixture, + config_dir: str, + mock_iot_domain_integration: Integration, + snapshot: SnapshotAssertion, +) -> None: + """Tests syntactic errors in individual packages.""" + + base_path = os.path.dirname(__file__) + hass.config.config_dir = os.path.join( + base_path, "fixtures", "core", "config", "package_schema_validation", config_dir + ) + + config = await config_util.async_hass_config_yaml(hass) + + error_records = [ + record.message + for record in caplog.get_records("call") + if record.levelno == logging.ERROR + ] + assert error_records == snapshot + + assert len(config["iot_domain"]) == 1 + + +@pytest.mark.parametrize( + "config_dir", + [ + "packages_is_a_list", + "packages_is_a_value", + "packages_is_null", + ], +) +async def test_packages_schema_validation_error( + hass: HomeAssistant, + caplog: pytest.LogCaptureFixture, + config_dir: str, + snapshot: SnapshotAssertion, +) -> None: + """Ensure that global package schema validation errors are logged.""" + + base_path = os.path.dirname(__file__) + hass.config.config_dir = os.path.join( + base_path, + "fixtures", + "core", + "config", + "package_schema_errors", + config_dir, + ) + + config = await config_util.async_hass_config_yaml(hass) + + error_records = [ + record.message + for record in caplog.get_records("call") + if record.levelno == logging.ERROR + ] + assert error_records == snapshot + + assert len(config[config_util.CONF_CORE][config_util.CONF_PACKAGES]) == 0 + + def test_extract_domain_configs() -> None: """Test the extraction of domain configuration.""" config = {