Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mqtt config validation error handling #103210

Merged
merged 5 commits into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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: 16 additions & 3 deletions homeassistant/components/mqtt/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,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 @@ -55,6 +56,7 @@
async_track_entity_registry_updated_event,
)
from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue
from homeassistant.helpers.json import json_bytes
from homeassistant.helpers.typing import (
UNDEFINED,
ConfigType,
Expand Down Expand Up @@ -207,6 +209,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 `diagnostic` or `None`."""
if (
CONF_ENTITY_CATEGORY in config
and config[CONF_ENTITY_CATEGORY] == EntityCategory.CONFIG
jbouwh marked this conversation as resolved.
Show resolved Hide resolved
):
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 +416,9 @@ 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)))
# Remove additional info from dict before dumping to YAML
yaml_config_str = yaml.dump(json_loads(json_bytes(yaml_config)))
jbouwh marked this conversation as resolved.
Show resolved Hide resolved
learn_more_url = (
f"https://www.home-assistant.io/integrations/{domain}.mqtt/"
)
Expand All @@ -427,7 +440,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
Loading