From 3bcc6194efd5b10866506b37ead68f393b5a61a9 Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Tue, 5 Dec 2023 15:07:32 +0100 Subject: [PATCH] Add domain key config validation (#104242) * Drop use of regex in helpers.extract_domain_configs * Update test * Revert test update * Add domain_from_config_key helper * Add validator * Address review comment * Update snapshots * Inline domain_from_config_key in validator --- homeassistant/bootstrap.py | 5 +- homeassistant/config.py | 32 ++++++++++-- homeassistant/helpers/check_config.py | 3 +- homeassistant/helpers/config_validation.py | 24 +++++++++ .../basic/configuration.yaml | 5 ++ .../basic_include/configuration.yaml | 3 ++ .../basic_include/integrations/.yaml | 0 .../basic_include/integrations/5.yaml | 0 .../integrations/iot_domain .yaml | 0 .../include_dir_list/invalid_domains/.yaml | 0 .../include_dir_list/invalid_domains/5.yaml | 0 .../invalid_domains/iot_domain .yaml | 0 .../packages/configuration.yaml | 7 +++ .../integrations/pack_5.yaml | 1 + .../integrations/pack_empty.yaml | 1 + .../integrations/pack_iot_domain_space.yaml | 1 + tests/helpers/test_config_validation.py | 16 ++++++ tests/snapshots/test_config.ambr | 51 +++++++++++++++++++ tests/test_config.py | 4 +- 19 files changed, 144 insertions(+), 9 deletions(-) create mode 100644 tests/fixtures/core/config/component_validation/basic_include/integrations/.yaml create mode 100644 tests/fixtures/core/config/component_validation/basic_include/integrations/5.yaml create mode 100644 tests/fixtures/core/config/component_validation/basic_include/integrations/iot_domain .yaml create mode 100644 tests/fixtures/core/config/component_validation/include_dir_list/invalid_domains/.yaml create mode 100644 tests/fixtures/core/config/component_validation/include_dir_list/invalid_domains/5.yaml create mode 100644 tests/fixtures/core/config/component_validation/include_dir_list/invalid_domains/iot_domain .yaml create mode 100644 tests/fixtures/core/config/component_validation/packages_include_dir_named/integrations/pack_5.yaml create mode 100644 tests/fixtures/core/config/component_validation/packages_include_dir_named/integrations/pack_empty.yaml create mode 100644 tests/fixtures/core/config/component_validation/packages_include_dir_named/integrations/pack_iot_domain_space.yaml diff --git a/homeassistant/bootstrap.py b/homeassistant/bootstrap.py index 0998ac6274c..83b2f18719f 100644 --- a/homeassistant/bootstrap.py +++ b/homeassistant/bootstrap.py @@ -27,6 +27,7 @@ from .const import ( from .exceptions import HomeAssistantError from .helpers import ( area_registry, + config_validation as cv, device_registry, entity, entity_registry, @@ -473,7 +474,9 @@ async def async_mount_local_lib_path(config_dir: str) -> str: def _get_domains(hass: core.HomeAssistant, config: dict[str, Any]) -> set[str]: """Get domains of components to set up.""" # Filter out the repeating and common config section [homeassistant] - domains = {key.partition(" ")[0] for key in config if key != core.DOMAIN} + domains = { + domain for key in config if (domain := cv.domain_key(key)) != core.DOMAIN + } # Add config entry domains if not hass.config.recovery_mode: diff --git a/homeassistant/config.py b/homeassistant/config.py index bbdd30c3683..6dd8bc21471 100644 --- a/homeassistant/config.py +++ b/homeassistant/config.py @@ -449,6 +449,19 @@ async def async_hass_config_yaml(hass: HomeAssistant) -> dict: base_exc.problem_mark.name = _relpath(hass, base_exc.problem_mark.name) raise + invalid_domains = [] + for key in config: + try: + cv.domain_key(key) + except vol.Invalid as exc: + suffix = "" + if annotation := find_annotation(config, exc.path): + suffix = f" at {_relpath(hass, annotation[0])}, line {annotation[1]}" + _LOGGER.error("Invalid domain '%s'%s", key, suffix) + invalid_domains.append(key) + for invalid_domain in invalid_domains: + config.pop(invalid_domain) + core_config = config.get(CONF_CORE, {}) await merge_packages_config(hass, config, core_config.get(CONF_PACKAGES, {})) return config @@ -982,9 +995,13 @@ async def merge_packages_config( for comp_name, comp_conf in pack_conf.items(): if comp_name == CONF_CORE: continue - # If component name is given with a trailing description, remove it - # when looking for component - domain = comp_name.partition(" ")[0] + try: + domain = cv.domain_key(comp_name) + except vol.Invalid: + _log_pkg_error( + hass, pack_name, comp_name, config, f"Invalid domain '{comp_name}'" + ) + continue try: integration = await async_get_integration_with_requirements( @@ -1263,8 +1280,13 @@ def extract_domain_configs(config: ConfigType, domain: str) -> Sequence[str]: Async friendly. """ - pattern = re.compile(rf"^{domain}(| .+)$") - return [key for key in config if pattern.match(key)] + domain_configs = [] + for key in config: + with suppress(vol.Invalid): + if cv.domain_key(key) != domain: + continue + domain_configs.append(key) + return domain_configs async def async_process_component_config( # noqa: C901 diff --git a/homeassistant/helpers/check_config.py b/homeassistant/helpers/check_config.py index 59334c20b30..1c8efadfdc5 100644 --- a/homeassistant/helpers/check_config.py +++ b/homeassistant/helpers/check_config.py @@ -31,6 +31,7 @@ from homeassistant.requirements import ( ) import homeassistant.util.yaml.loader as yaml_loader +from . import config_validation as cv from .typing import ConfigType @@ -175,7 +176,7 @@ async def async_check_ha_config_file( # noqa: C901 core_config.pop(CONF_PACKAGES, None) # Filter out repeating config sections - components = {key.partition(" ")[0] for key in config} + components = {cv.domain_key(key) for key in config} frontend_dependencies: set[str] = set() if "frontend" in components or "default_config" in components: diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index e07596ad450..e4b62dd679d 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -351,6 +351,30 @@ comp_entity_ids_or_uuids = vol.Any( ) +def domain_key(config_key: Any) -> str: + """Validate a top level config key with an optional label and return the domain. + + A domain is separated from a label by one or more spaces, empty labels are not + allowed. + + Examples: + 'hue' returns 'hue' + 'hue 1' returns 'hue' + 'hue 1' returns 'hue' + 'hue ' raises + 'hue ' raises + """ + if not isinstance(config_key, str): + raise vol.Invalid("invalid domain", path=[config_key]) + + parts = config_key.partition(" ") + _domain = parts[0] if parts[2].strip(" ") else config_key + if not _domain or _domain.strip(" ") != _domain: + raise vol.Invalid("invalid domain", path=[config_key]) + + return _domain + + def entity_domain(domain: str | list[str]) -> Callable[[Any], str]: """Validate that entity belong to domain.""" ent_domain = entities_domain(domain) diff --git a/tests/fixtures/core/config/component_validation/basic/configuration.yaml b/tests/fixtures/core/config/component_validation/basic/configuration.yaml index 9c3d1eb190b..49db89f45ba 100644 --- a/tests/fixtures/core/config/component_validation/basic/configuration.yaml +++ b/tests/fixtures/core/config/component_validation/basic/configuration.yaml @@ -56,3 +56,8 @@ custom_validator_bad_1: # This always raises ValueError custom_validator_bad_2: + +# Invalid domains +"iot_domain ": +"": +5: diff --git a/tests/fixtures/core/config/component_validation/basic_include/configuration.yaml b/tests/fixtures/core/config/component_validation/basic_include/configuration.yaml index 5744e3005fa..8e1c75c3511 100644 --- a/tests/fixtures/core/config/component_validation/basic_include/configuration.yaml +++ b/tests/fixtures/core/config/component_validation/basic_include/configuration.yaml @@ -8,3 +8,6 @@ custom_validator_ok_1: !include integrations/custom_validator_ok_1.yaml custom_validator_ok_2: !include integrations/custom_validator_ok_2.yaml custom_validator_bad_1: !include integrations/custom_validator_bad_1.yaml custom_validator_bad_2: !include integrations/custom_validator_bad_2.yaml +"iot_domain ": !include integrations/iot_domain .yaml +"": !include integrations/.yaml +5: !include integrations/5.yaml diff --git a/tests/fixtures/core/config/component_validation/basic_include/integrations/.yaml b/tests/fixtures/core/config/component_validation/basic_include/integrations/.yaml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/fixtures/core/config/component_validation/basic_include/integrations/5.yaml b/tests/fixtures/core/config/component_validation/basic_include/integrations/5.yaml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/fixtures/core/config/component_validation/basic_include/integrations/iot_domain .yaml b/tests/fixtures/core/config/component_validation/basic_include/integrations/iot_domain .yaml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/fixtures/core/config/component_validation/include_dir_list/invalid_domains/.yaml b/tests/fixtures/core/config/component_validation/include_dir_list/invalid_domains/.yaml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/fixtures/core/config/component_validation/include_dir_list/invalid_domains/5.yaml b/tests/fixtures/core/config/component_validation/include_dir_list/invalid_domains/5.yaml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/fixtures/core/config/component_validation/include_dir_list/invalid_domains/iot_domain .yaml b/tests/fixtures/core/config/component_validation/include_dir_list/invalid_domains/iot_domain .yaml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/fixtures/core/config/component_validation/packages/configuration.yaml b/tests/fixtures/core/config/component_validation/packages/configuration.yaml index b8116b5988e..25d734b126a 100644 --- a/tests/fixtures/core/config/component_validation/packages/configuration.yaml +++ b/tests/fixtures/core/config/component_validation/packages/configuration.yaml @@ -68,3 +68,10 @@ homeassistant: pack_custom_validator_bad_2: # This always raises ValueError custom_validator_bad_2: + # Invalid domains + pack_iot_domain_space: + "iot_domain ": + pack_empty: + "": + pack_5: + 5: diff --git a/tests/fixtures/core/config/component_validation/packages_include_dir_named/integrations/pack_5.yaml b/tests/fixtures/core/config/component_validation/packages_include_dir_named/integrations/pack_5.yaml new file mode 100644 index 00000000000..70bf80a6b64 --- /dev/null +++ b/tests/fixtures/core/config/component_validation/packages_include_dir_named/integrations/pack_5.yaml @@ -0,0 +1 @@ +5: diff --git a/tests/fixtures/core/config/component_validation/packages_include_dir_named/integrations/pack_empty.yaml b/tests/fixtures/core/config/component_validation/packages_include_dir_named/integrations/pack_empty.yaml new file mode 100644 index 00000000000..510d4682445 --- /dev/null +++ b/tests/fixtures/core/config/component_validation/packages_include_dir_named/integrations/pack_empty.yaml @@ -0,0 +1 @@ +"": diff --git a/tests/fixtures/core/config/component_validation/packages_include_dir_named/integrations/pack_iot_domain_space.yaml b/tests/fixtures/core/config/component_validation/packages_include_dir_named/integrations/pack_iot_domain_space.yaml new file mode 100644 index 00000000000..49b5720a536 --- /dev/null +++ b/tests/fixtures/core/config/component_validation/packages_include_dir_named/integrations/pack_iot_domain_space.yaml @@ -0,0 +1 @@ +"iot_domain ": diff --git a/tests/helpers/test_config_validation.py b/tests/helpers/test_config_validation.py index b44137e4f5c..f997e3a6c10 100644 --- a/tests/helpers/test_config_validation.py +++ b/tests/helpers/test_config_validation.py @@ -1631,3 +1631,19 @@ def test_platform_only_schema( cv.platform_only_config_schema("test_domain")({"test_domain": {"foo": "bar"}}) assert expected_message in caplog.text assert issue_registry.async_get_issue(HOMEASSISTANT_DOMAIN, expected_issue) + + +def test_domain() -> None: + """Test domain.""" + with pytest.raises(vol.Invalid): + cv.domain_key(5) + with pytest.raises(vol.Invalid): + cv.domain_key("") + with pytest.raises(vol.Invalid): + cv.domain_key("hue ") + with pytest.raises(vol.Invalid): + cv.domain_key("hue ") + assert cv.domain_key("hue") == "hue" + assert cv.domain_key("hue1") == "hue1" + assert cv.domain_key("hue 1") == "hue" + assert cv.domain_key("hue 1") == "hue" diff --git a/tests/snapshots/test_config.ambr b/tests/snapshots/test_config.ambr index 26a44f60184..76d3f0c4666 100644 --- a/tests/snapshots/test_config.ambr +++ b/tests/snapshots/test_config.ambr @@ -1,6 +1,18 @@ # serializer version: 1 # name: test_component_config_validation_error[basic] list([ + dict({ + 'has_exc_info': False, + 'message': "Invalid domain 'iot_domain ' at configuration.yaml, line 61", + }), + dict({ + 'has_exc_info': False, + 'message': "Invalid domain '' at configuration.yaml, line 62", + }), + dict({ + 'has_exc_info': False, + 'message': "Invalid domain '5' at configuration.yaml, line 1", + }), dict({ 'has_exc_info': False, 'message': "Invalid config for 'iot_domain' at configuration.yaml, line 6: required key 'platform' not provided", @@ -57,6 +69,18 @@ # --- # name: test_component_config_validation_error[basic_include] list([ + dict({ + 'has_exc_info': False, + 'message': "Invalid domain 'iot_domain ' at configuration.yaml, line 11", + }), + dict({ + 'has_exc_info': False, + 'message': "Invalid domain '' at configuration.yaml, line 12", + }), + dict({ + 'has_exc_info': False, + 'message': "Invalid domain '5' at configuration.yaml, line 1", + }), dict({ 'has_exc_info': False, 'message': "Invalid config for 'iot_domain' at integrations/iot_domain.yaml, line 5: required key 'platform' not provided", @@ -161,6 +185,18 @@ # --- # name: test_component_config_validation_error[packages] list([ + dict({ + 'has_exc_info': False, + 'message': "Setup of package 'pack_iot_domain_space' at configuration.yaml, line 72 failed: Invalid domain 'iot_domain '", + }), + dict({ + 'has_exc_info': False, + 'message': "Setup of package 'pack_empty' at configuration.yaml, line 74 failed: Invalid domain ''", + }), + dict({ + 'has_exc_info': False, + 'message': "Setup of package 'pack_5' at configuration.yaml, line 76 failed: Invalid domain '5'", + }), dict({ 'has_exc_info': False, 'message': "Invalid config for 'iot_domain' at configuration.yaml, line 11: required key 'platform' not provided", @@ -217,6 +253,18 @@ # --- # name: test_component_config_validation_error[packages_include_dir_named] list([ + dict({ + 'has_exc_info': False, + 'message': "Setup of package 'pack_5' at integrations/pack_5.yaml, line 1 failed: Invalid domain '5'", + }), + dict({ + 'has_exc_info': False, + 'message': "Setup of package 'pack_empty' at integrations/pack_empty.yaml, line 1 failed: Invalid domain ''", + }), + dict({ + 'has_exc_info': False, + 'message': "Setup of package 'pack_iot_domain_space' at integrations/pack_iot_domain_space.yaml, line 1 failed: Invalid domain 'iot_domain '", + }), dict({ 'has_exc_info': False, 'message': "Invalid config for 'adr_0007_2' at integrations/adr_0007_2.yaml, line 2: required key 'host' not provided", @@ -273,6 +321,9 @@ # --- # name: test_component_config_validation_error_with_docs[basic] list([ + "Invalid domain 'iot_domain ' at configuration.yaml, line 61", + "Invalid domain '' at configuration.yaml, line 62", + "Invalid domain '5' at configuration.yaml, line 1", "Invalid config for 'iot_domain' at configuration.yaml, line 6: required key 'platform' not provided, please check the docs at https://www.home-assistant.io/integrations/iot_domain", "Invalid config for 'iot_domain' from integration 'non_adr_0007' at configuration.yaml, line 9: expected str for dictionary value 'option1', got 123, please check the docs at https://www.home-assistant.io/integrations/non_adr_0007", "Invalid config for 'iot_domain' from integration 'non_adr_0007' at configuration.yaml, line 12: 'no_such_option' is an invalid option for 'non_adr_0007.iot_domain', check: no_such_option, please check the docs at https://www.home-assistant.io/integrations/non_adr_0007", diff --git a/tests/test_config.py b/tests/test_config.py index 1e309e2908f..8ec509cd895 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -2043,7 +2043,7 @@ async def test_component_config_validation_error( for domain_with_label in config: integration = await async_get_integration( - hass, domain_with_label.partition(" ")[0] + hass, cv.domain_key(domain_with_label) ) await config_util.async_process_component_and_handle_errors( hass, @@ -2088,7 +2088,7 @@ async def test_component_config_validation_error_with_docs( for domain_with_label in config: integration = await async_get_integration( - hass, domain_with_label.partition(" ")[0] + hass, cv.domain_key(domain_with_label) ) await config_util.async_process_component_and_handle_errors( hass,