mirror of
https://github.com/home-assistant/supervisor.git
synced 2026-10-10 03:59:41 +01:00
Guard restored discovery messages against uuid and service collisions
Discovery messages are keyed by uuid across all apps, so restoring a message whose uuid is already taken silently dropped the message it collided with, including one belonging to another app. Skip such a message instead, and log it as it points at a corrupt backup. The set of services which already have a message was also built once before the loop, so a backup listing the same service twice ended up with two messages for it. Only one of the two can ever be updated or removed again, since send() and remove() match on app and service, while both are handed to Home Assistant. Grow the set while restoring so that the first message for a service wins. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
f7f73af38e
commit
ace2ec2cf5
2 files changed
+78
-2
No files matched your search
@@ -142,11 +142,15 @@ class Discovery(CoreSysAttributes, FileConfiguration):
|
||||
is not started again. Instead it is marked as unannounced so that the
|
||||
push happens once the app sends the message itself.
|
||||
"""
|
||||
live_services = {message.service for message in self.messages_for_app(app.slug)}
|
||||
# Services which already have a message, grown while restoring so that
|
||||
# a backup listing a service twice does not end up with two messages
|
||||
known_services = {
|
||||
message.service for message in self.messages_for_app(app.slug)
|
||||
}
|
||||
restored = False
|
||||
|
||||
for message in messages:
|
||||
if message.service in live_services:
|
||||
if message.service in known_services:
|
||||
continue
|
||||
if message.service not in app.discovery:
|
||||
_LOGGER.info(
|
||||
@@ -155,9 +159,20 @@ class Discovery(CoreSysAttributes, FileConfiguration):
|
||||
app.slug,
|
||||
)
|
||||
continue
|
||||
if message.uuid in self.message_obj:
|
||||
# Messages are keyed by uuid across all apps, so restoring this
|
||||
# one would drop the message it collides with
|
||||
_LOGGER.warning(
|
||||
"Skipping discovery message for service %s of app %s, uuid %s is already in use",
|
||||
message.service,
|
||||
app.slug,
|
||||
message.uuid,
|
||||
)
|
||||
continue
|
||||
|
||||
self.message_obj[message.uuid] = message
|
||||
self._unannounced.add(message.uuid)
|
||||
known_services.add(message.service)
|
||||
restored = True
|
||||
_LOGGER.info(
|
||||
"Restored discovery %s for service %s from %s",
|
||||
|
||||
@@ -131,6 +131,67 @@ async def test_restore_app_messages_skips_dropped_service(
|
||||
assert "app local_ssh does not provide it anymore" in caplog.text
|
||||
|
||||
|
||||
async def test_restore_app_messages_skips_uuid_in_use(
|
||||
coresys: CoreSys, app_with_discovery: App, caplog: pytest.LogCaptureFixture
|
||||
):
|
||||
"""Test a message does not overwrite the message another app owns.
|
||||
|
||||
Messages are keyed by uuid across all apps, so restoring a uuid which is
|
||||
already taken would drop the message it collides with.
|
||||
"""
|
||||
other = Message(app="core_mosquitto", service="mqtt", config={}, uuid=BACKUP_UUID)
|
||||
coresys.discovery.message_obj[other.uuid] = other
|
||||
|
||||
await coresys.discovery.restore_app_messages(
|
||||
app_with_discovery,
|
||||
[
|
||||
Message(
|
||||
app=app_with_discovery.slug,
|
||||
service="mcp",
|
||||
config=MCP_CONFIG,
|
||||
uuid=BACKUP_UUID,
|
||||
)
|
||||
],
|
||||
)
|
||||
|
||||
assert coresys.discovery.get(BACKUP_UUID) is other
|
||||
assert coresys.discovery.messages_for_app(app_with_discovery.slug) == []
|
||||
assert f"uuid {BACKUP_UUID} is already in use" in caplog.text
|
||||
|
||||
|
||||
async def test_restore_app_messages_service_only_once(
|
||||
coresys: CoreSys, app_with_discovery: App
|
||||
):
|
||||
"""Test a service repeated in a backup only gets a single message.
|
||||
|
||||
Two messages for one service would both be handed to Home Assistant, and
|
||||
only one of them can ever be updated or removed again, as send() and
|
||||
remove() match on app and service.
|
||||
"""
|
||||
await coresys.discovery.restore_app_messages(
|
||||
app_with_discovery,
|
||||
[
|
||||
Message(
|
||||
app=app_with_discovery.slug,
|
||||
service="mcp",
|
||||
config=MCP_CONFIG,
|
||||
uuid=BACKUP_UUID,
|
||||
),
|
||||
Message(
|
||||
app=app_with_discovery.slug,
|
||||
service="mcp",
|
||||
config={"url": "http://local-ssh:9999/mcp"},
|
||||
uuid="a1b2c3d4e5f60718293a4b5c6d7e8f90",
|
||||
),
|
||||
],
|
||||
)
|
||||
|
||||
assert [
|
||||
message.uuid
|
||||
for message in coresys.discovery.messages_for_app(app_with_discovery.slug)
|
||||
] == [BACKUP_UUID]
|
||||
|
||||
|
||||
async def test_messages_for_app(coresys: CoreSys, app_with_discovery: App):
|
||||
"""Test listing the messages of a single app."""
|
||||
message = await coresys.discovery.send(app_with_discovery, "mcp", dict(MCP_CONFIG))
|
||||
|
||||
Reference in new issue
Block a user