From 9b21a48048adb753e2085bdf7d33d4cb735607fb Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Fri, 1 Apr 2022 17:11:31 +0200 Subject: [PATCH] Mend incorrectly imported MQTT config entries (#68987) Co-authored-by: Paulus Schoutsen --- homeassistant/components/mqtt/__init__.py | 74 +++++++++++++++++++---- tests/components/mqtt/test_init.py | 53 +++++++++++++++- 2 files changed, 114 insertions(+), 13 deletions(-) diff --git a/homeassistant/components/mqtt/__init__.py b/homeassistant/components/mqtt/__init__.py index 22d99d1b7be..7c16da7f2aa 100644 --- a/homeassistant/components/mqtt/__init__.py +++ b/homeassistant/components/mqtt/__init__.py @@ -131,12 +131,15 @@ DEFAULT_PROTOCOL = PROTOCOL_311 DEFAULT_TLS_PROTOCOL = "auto" DEFAULT_VALUES = { - CONF_PORT: DEFAULT_PORT, - CONF_WILL_MESSAGE: DEFAULT_WILL, CONF_BIRTH_MESSAGE: DEFAULT_BIRTH, CONF_DISCOVERY: DEFAULT_DISCOVERY, + CONF_PORT: DEFAULT_PORT, + CONF_TLS_VERSION: DEFAULT_TLS_PROTOCOL, + CONF_WILL_MESSAGE: DEFAULT_WILL, } +MANDATORY_DEFAULT_VALUES = (CONF_PORT,) + ATTR_TOPIC_TEMPLATE = "topic_template" ATTR_PAYLOAD_TEMPLATE = "payload_template" @@ -203,9 +206,7 @@ CONFIG_SCHEMA_BASE = vol.Schema( CONF_CLIENT_CERT, "client_key_auth", msg=CLIENT_KEY_AUTH_MSG ): cv.isfile, vol.Optional(CONF_TLS_INSECURE): cv.boolean, - vol.Optional(CONF_TLS_VERSION, default=DEFAULT_TLS_PROTOCOL): vol.Any( - "auto", "1.0", "1.1", "1.2" - ), + vol.Optional(CONF_TLS_VERSION): vol.Any("auto", "1.0", "1.1", "1.2"), vol.Optional(CONF_PROTOCOL, default=DEFAULT_PROTOCOL): vol.All( cv.string, vol.In([PROTOCOL_31, PROTOCOL_311]) ), @@ -220,6 +221,17 @@ CONFIG_SCHEMA_BASE = vol.Schema( } ) +DEPRECATED_CONFIG_KEYS = [ + CONF_BIRTH_MESSAGE, + CONF_BROKER, + CONF_DISCOVERY, + CONF_PASSWORD, + CONF_PORT, + CONF_TLS_VERSION, + CONF_USERNAME, + CONF_WILL_MESSAGE, +] + CONFIG_SCHEMA = vol.Schema( { DOMAIN: vol.All( @@ -602,6 +614,9 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: hass.data[DATA_MQTT_CONFIG] = conf if not bool(hass.config_entries.async_entries(DOMAIN)): + # Create an import flow if the user has yaml configured entities etc. + # but no broker configuration. Note: The intention is not for this to + # import broker configuration from YAML because that has been deprecated. hass.async_create_task( hass.config_entries.flow.async_init( DOMAIN, @@ -612,18 +627,53 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: return True -def _merge_config(entry, conf): - """Merge configuration.yaml config with config entry.""" - # Base config on default values +def _merge_basic_config( + hass: HomeAssistant, entry: ConfigEntry, yaml_config: dict[str, Any] +) -> None: + """Merge basic options in configuration.yaml config with config entry. + + This mends incomplete migration from old version of HA Core. + """ + + entry_updated = False + entry_config = {**entry.data} + for key in DEPRECATED_CONFIG_KEYS: + if key in yaml_config and key not in entry_config: + entry_config[key] = yaml_config[key] + entry_updated = True + + for key in MANDATORY_DEFAULT_VALUES: + if key not in entry_config: + entry_config[key] = DEFAULT_VALUES[key] + entry_updated = True + + if entry_updated: + hass.config_entries.async_update_entry(entry, data=entry_config) + + +def _merge_extended_config(entry, conf): + """Merge advanced options in configuration.yaml config with config entry.""" + # Add default values conf = {**DEFAULT_VALUES, **conf} return {**conf, **entry.data} async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Load a config entry.""" - # If user didn't have configuration.yaml config, generate defaults + # Merge basic configuration, and add missing defaults for basic options + _merge_basic_config(hass, entry, hass.data.get(DATA_MQTT_CONFIG, {})) + + # Bail out if broker setting is missing + if CONF_BROKER not in entry.data: + _LOGGER.error("MQTT broker is not configured, please configure it") + return False + + # If user doesn't have configuration.yaml config, generate default values + # for options not in config entry data if (conf := hass.data.get(DATA_MQTT_CONFIG)) is None: conf = CONFIG_SCHEMA_BASE(dict(entry.data)) + + # User has configuration.yaml config, warn about config entry overrides elif any(key in conf for key in entry.data): shared_keys = conf.keys() & entry.data.keys() override = {k: entry.data[k] for k in shared_keys} @@ -635,8 +685,8 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: override, ) - # Merge the configuration values from configuration.yaml - conf = _merge_config(entry, conf) + # Merge advanced configuration values from configuration.yaml + conf = _merge_extended_config(entry, conf) hass.data[DATA_MQTT] = MQTT( hass, @@ -870,7 +920,7 @@ class MQTT: if (conf := hass.data.get(DATA_MQTT_CONFIG)) is None: conf = CONFIG_SCHEMA_BASE(dict(entry.data)) - self.conf = _merge_config(entry, conf) + self.conf = _merge_extended_config(entry, conf) await self.async_disconnect() self.init_client() await self.async_connect() diff --git a/tests/components/mqtt/test_init.py b/tests/components/mqtt/test_init.py index 87eddd4421f..03874ed331e 100644 --- a/tests/components/mqtt/test_init.py +++ b/tests/components/mqtt/test_init.py @@ -23,7 +23,7 @@ from homeassistant.const import ( TEMP_CELSIUS, ) import homeassistant.core as ha -from homeassistant.core import CoreState, callback +from homeassistant.core import CoreState, HomeAssistant, callback from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import device_registry as dr, template from homeassistant.helpers.entity import Entity @@ -1620,6 +1620,57 @@ async def test_setup_entry_with_config_override(hass, device_reg, mqtt_client_mo assert device_entry is not None +async def test_update_incomplete_entry( + hass: HomeAssistant, device_reg, mqtt_client_mock, caplog +): + """Test if the MQTT component loads when config entry data is incomplete.""" + data = ( + '{ "device":{"identifiers":["0AFFD2"]},' + ' "state_topic": "foobar/sensor",' + ' "unique_id": "unique" }' + ) + + # Config entry data is incomplete + entry = MockConfigEntry(domain=mqtt.DOMAIN, data={"port": 1234}) + entry.add_to_hass(hass) + # Mqtt present in yaml config + config = {"broker": "yaml_broker"} + await async_setup_component(hass, mqtt.DOMAIN, {mqtt.DOMAIN: config}) + await hass.async_block_till_done() + + # Config entry data should now be updated + assert entry.data == { + "port": 1234, + "broker": "yaml_broker", + } + # Warnings about broker deprecated, but not about other keys with default values + assert ( + "The 'broker' option is deprecated, please remove it from your configuration" + in caplog.text + ) + assert ( + "Deprecated configuration settings found in configuration.yaml. These settings " + "from your configuration entry will override: {'broker': 'yaml_broker'}" + in caplog.text + ) + + # Discover a device to verify the entry was setup correctly + async_fire_mqtt_message(hass, "homeassistant/sensor/bla/config", data) + await hass.async_block_till_done() + + device_entry = device_reg.async_get_device({("mqtt", "0AFFD2")}) + assert device_entry is not None + + +async def test_fail_no_broker(hass, device_reg, mqtt_client_mock, caplog): + """Test if the MQTT component loads when broker configuration is missing.""" + # Config entry data is incomplete + entry = MockConfigEntry(domain=mqtt.DOMAIN, data={}) + entry.add_to_hass(hass) + assert not await hass.config_entries.async_setup(entry.entry_id) + assert "MQTT broker is not configured, please configure it" in caplog.text + + @pytest.mark.no_fail_on_log_exception async def test_message_callback_exception_gets_logged(hass, caplog, mqtt_mock): """Test exception raised by message handler."""