List discovery only includes healthy addons (#4451)

Co-authored-by: Stefan Agner <stefan@agner.ch>

supervisor/api/discovery.py

@@ -12,6 +12,7 @@ from ..const import (
     ATTR_SERVICES,
     ATTR_UUID,
     REQUEST_FROM,
+    AddonState,
 )
 from ..coresys import CoreSysAttributes
 from ..discovery.validate import valid_discovery_service
@@ -41,19 +42,19 @@ class APIDiscovery(CoreSysAttributes):
     @api_process
     @require_home_assistant
     async def list(self, request):
-        """Show register services."""
+        """Show registered and available services."""
-
         # Get available discovery
-        discovery = []
+        discovery = [
-        for message in self.sys_discovery.list_messages:
+            {
-            discovery.append(
+                ATTR_ADDON: message.addon,
-                {
+                ATTR_SERVICE: message.service,
-                    ATTR_ADDON: message.addon,
+                ATTR_UUID: message.uuid,
-                    ATTR_SERVICE: message.service,
+                ATTR_CONFIG: message.config,
-                    ATTR_UUID: message.uuid,
+            }
-                    ATTR_CONFIG: message.config,
+            for message in self.sys_discovery.list_messages
-                }
+            if (addon := self.sys_addons.get(message.addon, local_only=True))
-            )
+            and addon.state == AddonState.STARTED
+        ]
 
         # Get available services/add-ons
         services = {}

tests/api/test_discovery.py

@@ -1,18 +1,22 @@
 """Test discovery API."""
 
 import logging
-from unittest.mock import MagicMock, patch
+from unittest.mock import ANY, MagicMock, patch
 from uuid import uuid4
 
 from aiohttp.test_utils import TestClient
 import pytest
 
 from supervisor.addons.addon import Addon
-from supervisor.discovery import Discovery
+from supervisor.const import AddonState
+from supervisor.coresys import CoreSys
+from supervisor.discovery import Discovery, Message
+
+from tests.common import load_json_fixture
 
 
 @pytest.mark.parametrize("api_client", ["local_ssh"], indirect=True)
-async def test_discovery_forbidden(
+async def test_api_discovery_forbidden(
     api_client: TestClient, caplog: pytest.LogCaptureFixture, install_addon_ssh
 ):
     """Test addon sending discovery message for an unregistered service."""
@@ -32,7 +36,7 @@ async def test_discovery_forbidden(
 
 
 @pytest.mark.parametrize("api_client", ["local_ssh"], indirect=True)
-async def test_discovery_unknown_service(
+async def test_api_discovery_unknown_service(
     api_client: TestClient, caplog: pytest.LogCaptureFixture, install_addon_ssh: Addon
 ):
     """Test addon sending discovery message for an unkown service."""
@@ -51,3 +55,45 @@ async def test_discovery_unknown_service(
     result = await resp.json()
     assert result["data"]["uuid"] == message.uuid
     assert "Please report this to the maintainer of the add-on" in caplog.text
+
+
+@pytest.mark.parametrize(
+    "skip_state", [AddonState.ERROR, AddonState.STOPPED, AddonState.STARTUP]
+)
+async def test_api_list_discovery(
+    api_client: TestClient,
+    coresys: CoreSys,
+    install_addon_ssh: Addon,
+    skip_state: AddonState,
+):
+    """Test listing discovery messages only returns ones for healthy services."""
+    with patch(
+        "supervisor.utils.common.read_json_or_yaml_file",
+        return_value=load_json_fixture("discovery.json"),
+    ), patch("supervisor.utils.common.Path.is_file", return_value=True):
+        coresys.discovery.read_data()
+
+    await coresys.discovery.load()
+    assert coresys.discovery.list_messages == [
+        Message(addon="core_mosquitto", service="mqtt", config=ANY, uuid=ANY),
+        Message(addon="local_ssh", service="adguard", config=ANY, uuid=ANY),
+    ]
+
+    install_addon_ssh.state = AddonState.STARTED
+    resp = await api_client.get("/discovery")
+    assert resp.status == 200
+    result = await resp.json()
+    assert result["data"]["discovery"] == [
+        {
+            "addon": "local_ssh",
+            "service": "adguard",
+            "config": ANY,
+            "uuid": ANY,
+        }
+    ]
+
+    install_addon_ssh.state = skip_state
+    resp = await api_client.get("/discovery")
+    assert resp.status == 200
+    result = await resp.json()
+    assert result["data"]["discovery"] == []

tests/fixtures/discovery.json

@@ -0,0 +1,26 @@
+{
+  "discovery": [
+    {
+      "addon": "core_mosquitto",
+      "service": "mqtt",
+      "config": {
+        "host": "core-mosquitto",
+        "port": 1883,
+        "ssl": false,
+        "protocol": "3.1.1",
+        "username": "homeassistant",
+        "password": "password"
+      },
+      "uuid": "0c83a27125fe4421b58ea28eef5834c5"
+    },
+    {
+      "addon": "local_ssh",
+      "service": "adguard",
+      "config": {
+        "host": "127.0.0.1",
+        "port": 45158
+      },
+      "uuid": "0658be435a2948ec8b4d706d6708c56e"
+    }
+  ]
+}