Skip to content

Commit

Permalink
Fix mqtt config validation error handling (#103210)
Browse files Browse the repository at this point in the history
* Fix MQTT config check

* Fix handling invalid enity_category for sensors

* Improve docstr

* Update comment

* Use correct util for yaml dump
  • Loading branch information
jbouwh authored Nov 2, 2023
1 parent fe482af commit 4a4d2ad
Show file tree
Hide file tree
Showing 12 changed files with 80 additions and 25 deletions.
10 changes: 8 additions & 2 deletions homeassistant/components/mqtt/binary_sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
MqttAvailability,
MqttEntity,
async_setup_entity_entry_helper,
validate_sensor_entity_category,
write_state_on_attr_change,
)
from .models import MqttValueTemplate, ReceiveMessage
Expand All @@ -55,7 +56,7 @@
DEFAULT_FORCE_UPDATE = False
CONF_EXPIRE_AFTER = "expire_after"

PLATFORM_SCHEMA_MODERN = MQTT_RO_SCHEMA.extend(
_PLATFORM_SCHEMA_BASE = MQTT_RO_SCHEMA.extend(
{
vol.Optional(CONF_DEVICE_CLASS): vol.Any(DEVICE_CLASSES_SCHEMA, None),
vol.Optional(CONF_EXPIRE_AFTER): cv.positive_int,
Expand All @@ -67,7 +68,12 @@
}
).extend(MQTT_ENTITY_COMMON_SCHEMA.schema)

DISCOVERY_SCHEMA = PLATFORM_SCHEMA_MODERN.extend({}, extra=vol.REMOVE_EXTRA)
DISCOVERY_SCHEMA = vol.All(
validate_sensor_entity_category,
_PLATFORM_SCHEMA_BASE.extend({}, extra=vol.REMOVE_EXTRA),
)

PLATFORM_SCHEMA_MODERN = vol.All(validate_sensor_entity_category, _PLATFORM_SCHEMA_BASE)


async def async_setup_entry(
Expand Down
6 changes: 3 additions & 3 deletions homeassistant/components/mqtt/climate.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,16 +232,16 @@
def valid_preset_mode_configuration(config: ConfigType) -> ConfigType:
"""Validate that the preset mode reset payload is not one of the preset modes."""
if PRESET_NONE in config[CONF_PRESET_MODES_LIST]:
raise ValueError("preset_modes must not include preset mode 'none'")
raise vol.Invalid("preset_modes must not include preset mode 'none'")
return config


def valid_humidity_range_configuration(config: ConfigType) -> ConfigType:
"""Validate a target_humidity range configuration, throws otherwise."""
if config[CONF_HUMIDITY_MIN] >= config[CONF_HUMIDITY_MAX]:
raise ValueError("target_humidity_max must be > target_humidity_min")
raise vol.Invalid("target_humidity_max must be > target_humidity_min")
if config[CONF_HUMIDITY_MAX] > 100:
raise ValueError("max_humidity must be <= 100")
raise vol.Invalid("max_humidity must be <= 100")

return config

Expand Down
6 changes: 3 additions & 3 deletions homeassistant/components/mqtt/fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,16 +116,16 @@
def valid_speed_range_configuration(config: ConfigType) -> ConfigType:
"""Validate that the fan speed_range configuration is valid, throws if it isn't."""
if config[CONF_SPEED_RANGE_MIN] == 0:
raise ValueError("speed_range_min must be > 0")
raise vol.Invalid("speed_range_min must be > 0")
if config[CONF_SPEED_RANGE_MIN] >= config[CONF_SPEED_RANGE_MAX]:
raise ValueError("speed_range_max must be > speed_range_min")
raise vol.Invalid("speed_range_max must be > speed_range_min")
return config


def valid_preset_mode_configuration(config: ConfigType) -> ConfigType:
"""Validate that the preset mode reset payload is not one of the preset modes."""
if config[CONF_PAYLOAD_RESET_PRESET_MODE] in config[CONF_PRESET_MODES_LIST]:
raise ValueError("preset_modes must not contain payload_reset_preset_mode")
raise vol.Invalid("preset_modes must not contain payload_reset_preset_mode")
return config


Expand Down
6 changes: 3 additions & 3 deletions homeassistant/components/mqtt/humidifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
def valid_mode_configuration(config: ConfigType) -> ConfigType:
"""Validate that the mode reset payload is not one of the available modes."""
if config[CONF_PAYLOAD_RESET_MODE] in config[CONF_AVAILABLE_MODES_LIST]:
raise ValueError("modes must not contain payload_reset_mode")
raise vol.Invalid("modes must not contain payload_reset_mode")
return config


Expand All @@ -113,9 +113,9 @@ def valid_humidity_range_configuration(config: ConfigType) -> ConfigType:
throws if it isn't.
"""
if config[CONF_TARGET_HUMIDITY_MIN] >= config[CONF_TARGET_HUMIDITY_MAX]:
raise ValueError("target_humidity_max must be > target_humidity_min")
raise vol.Invalid("target_humidity_max must be > target_humidity_min")
if config[CONF_TARGET_HUMIDITY_MAX] > 100:
raise ValueError("max_humidity must be <= 100")
raise vol.Invalid("max_humidity must be <= 100")

return config

Expand Down
19 changes: 15 additions & 4 deletions homeassistant/components/mqtt/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from typing import TYPE_CHECKING, Any, Protocol, cast, final

import voluptuous as vol
import yaml

from homeassistant.config_entries import ConfigEntry
from homeassistant.const import (
Expand All @@ -28,6 +27,7 @@
CONF_NAME,
CONF_UNIQUE_ID,
CONF_VALUE_TEMPLATE,
EntityCategory,
)
from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback
from homeassistant.helpers import (
Expand Down Expand Up @@ -63,6 +63,7 @@
UndefinedType,
)
from homeassistant.util.json import json_loads
from homeassistant.util.yaml import dump as yaml_dump

from . import debug_info, subscription
from .client import async_publish
Expand Down Expand Up @@ -207,6 +208,16 @@ def validate_device_has_at_least_one_identifier(value: ConfigType) -> ConfigType
)


def validate_sensor_entity_category(config: ConfigType) -> ConfigType:
"""Check the sensor's entity category is not set to `config` which is invalid for sensors."""
if (
CONF_ENTITY_CATEGORY in config
and config[CONF_ENTITY_CATEGORY] == EntityCategory.CONFIG
):
raise vol.Invalid("Entity category `config` is invalid")
return config


MQTT_ENTITY_DEVICE_INFO_SCHEMA = vol.All(
cv.deprecated(CONF_DEPRECATED_VIA_HUB, CONF_VIA_DEVICE),
vol.Schema(
Expand Down Expand Up @@ -404,8 +415,8 @@ def _async_setup_entities() -> None:
error = str(ex)
config_file = getattr(yaml_config, "__config_file__", "?")
line = getattr(yaml_config, "__line__", "?")
issue_id = hex(hash(frozenset(yaml_config.items())))
yaml_config_str = yaml.dump(dict(yaml_config))
issue_id = hex(hash(frozenset(yaml_config)))
yaml_config_str = yaml_dump(yaml_config)
learn_more_url = (
f"https://www.home-assistant.io/integrations/{domain}.mqtt/"
)
Expand All @@ -427,7 +438,7 @@ def _async_setup_entities() -> None:
translation_key="invalid_platform_config",
)
_LOGGER.error(
"%s for manual configured MQTT %s item, in %s, line %s Got %s",
"%s for manually configured MQTT %s item, in %s, line %s Got %s",
error,
domain,
config_file,
Expand Down
4 changes: 3 additions & 1 deletion homeassistant/components/mqtt/sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
MqttAvailability,
MqttEntity,
async_setup_entity_entry_helper,
validate_sensor_entity_category,
write_state_on_attr_change,
)
from .models import (
Expand All @@ -70,7 +71,6 @@
DEFAULT_NAME = "MQTT Sensor"
DEFAULT_FORCE_UPDATE = False


_PLATFORM_SCHEMA_BASE = MQTT_RO_SCHEMA.extend(
{
vol.Optional(CONF_DEVICE_CLASS): vol.Any(DEVICE_CLASSES_SCHEMA, None),
Expand All @@ -88,13 +88,15 @@
# Deprecated in HA Core 2021.11.0 https://github.com/home-assistant/core/pull/54840
# Removed in HA Core 2023.6.0
cv.removed(CONF_LAST_RESET_TOPIC),
validate_sensor_entity_category,
_PLATFORM_SCHEMA_BASE,
)

DISCOVERY_SCHEMA = vol.All(
# Deprecated in HA Core 2021.11.0 https://github.com/home-assistant/core/pull/54840
# Removed in HA Core 2023.6.0
cv.removed(CONF_LAST_RESET_TOPIC),
validate_sensor_entity_category,
_PLATFORM_SCHEMA_BASE.extend({}, extra=vol.REMOVE_EXTRA),
)

Expand Down
4 changes: 2 additions & 2 deletions homeassistant/components/mqtt/text.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@
def valid_text_size_configuration(config: ConfigType) -> ConfigType:
"""Validate that the text length configuration is valid, throws if it isn't."""
if config[CONF_MIN] >= config[CONF_MAX]:
raise ValueError("text length min must be >= max")
raise vol.Invalid("text length min must be >= max")
if config[CONF_MAX] > MAX_LENGTH_STATE_STATE:
raise ValueError(f"max text length must be <= {MAX_LENGTH_STATE_STATE}")
raise vol.Invalid(f"max text length must be <= {MAX_LENGTH_STATE_STATE}")

return config

Expand Down
2 changes: 1 addition & 1 deletion tests/components/mqtt/test_alarm_control_panel.py
Original file line number Diff line number Diff line change
Expand Up @@ -1297,7 +1297,7 @@ async def test_reload_after_invalid_config(
assert hass.states.get("alarm_control_panel.test") is None
assert (
"extra keys not allowed @ data['invalid_topic'] for "
"manual configured MQTT alarm_control_panel item, "
"manually configured MQTT alarm_control_panel item, "
"in ?, line ? Got {'name': 'test', 'invalid_topic': 'test-topic'}"
in caplog.text
)
Expand Down
2 changes: 1 addition & 1 deletion tests/components/mqtt/test_climate.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ async def test_preset_none_in_preset_modes(
) -> None:
"""Test the preset mode payload reset configuration."""
assert await mqtt_mock_entry()
assert "not a valid value" in caplog.text
assert "preset_modes must not include preset mode 'none'" in caplog.text


@pytest.mark.parametrize(
Expand Down
4 changes: 2 additions & 2 deletions tests/components/mqtt/test_fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -1788,7 +1788,7 @@ async def test_attributes(
},
False,
None,
"not a valid value",
"speed_range_max must be > speed_range_min",
),
(
"test14",
Expand All @@ -1805,7 +1805,7 @@ async def test_attributes(
},
False,
None,
"not a valid value",
"speed_range_min must be > 0",
),
(
"test15",
Expand Down
38 changes: 37 additions & 1 deletion tests/components/mqtt/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -2134,7 +2134,7 @@ async def test_setup_manual_mqtt_with_platform_key(
"""Test set up a manual MQTT item with a platform key."""
assert await mqtt_mock_entry()
assert (
"extra keys not allowed @ data['platform'] for manual configured MQTT light item"
"extra keys not allowed @ data['platform'] for manually configured MQTT light item"
in caplog.text
)

Expand All @@ -2151,6 +2151,42 @@ async def test_setup_manual_mqtt_with_invalid_config(
assert "required key not provided" in caplog.text


@pytest.mark.parametrize(
"hass_config",
[
{
mqtt.DOMAIN: {
"sensor": {
"name": "test",
"state_topic": "test-topic",
"entity_category": "config",
}
}
},
{
mqtt.DOMAIN: {
"binary_sensor": {
"name": "test",
"state_topic": "test-topic",
"entity_category": "config",
}
}
},
],
)
@patch(
"homeassistant.components.mqtt.PLATFORMS", [Platform.BINARY_SENSOR, Platform.SENSOR]
)
async def test_setup_manual_mqtt_with_invalid_entity_category(
hass: HomeAssistant,
mqtt_mock_entry: MqttMockHAClientGenerator,
caplog: pytest.LogCaptureFixture,
) -> None:
"""Test set up a manual sensor item with an invalid entity category."""
assert await mqtt_mock_entry()
assert "Entity category `config` is invalid" in caplog.text


@patch("homeassistant.components.mqtt.PLATFORMS", [])
@pytest.mark.parametrize(
("mqtt_config_entry_data", "protocol"),
Expand Down
4 changes: 2 additions & 2 deletions tests/components/mqtt/test_text.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ async def test_attribute_validation_max_greater_then_min(
) -> None:
"""Test the validation of min and max configuration attributes."""
assert await mqtt_mock_entry()
assert "not a valid value" in caplog.text
assert "text length min must be >= max" in caplog.text


@pytest.mark.parametrize(
Expand All @@ -236,7 +236,7 @@ async def test_attribute_validation_max_not_greater_then_max_state_length(
) -> None:
"""Test the max value of of max configuration attribute."""
assert await mqtt_mock_entry()
assert "not a valid value" in caplog.text
assert "max text length must be <= 255" in caplog.text


@pytest.mark.parametrize(
Expand Down

0 comments on commit 4a4d2ad

Please sign in to comment.