From e60af93e2bf79f4296baf8717f455a3b9aec5238 Mon Sep 17 00:00:00 2001
From: Mike Degatano <michael.degatano@gmail.com>
Date: Sat, 22 Jul 2023 17:27:42 -0400
Subject: [PATCH] List discovery only includes healthy addons (#4451)

Co-authored-by: Stefan Agner <stefan@agner.ch>
---
 supervisor/api/discovery.py   | 25 ++++++++--------
 tests/api/test_discovery.py   | 54 ++++++++++++++++++++++++++++++++---
 tests/fixtures/discovery.json | 26 +++++++++++++++++
 3 files changed, 89 insertions(+), 16 deletions(-)
 create mode 100644 tests/fixtures/discovery.json

diff --git a/supervisor/api/discovery.py b/supervisor/api/discovery.py
index a095ca14b..f3365dd83 100644
--- a/supervisor/api/discovery.py
+++ b/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 = []
-        for message in self.sys_discovery.list_messages:
-            discovery.append(
-                {
-                    ATTR_ADDON: message.addon,
-                    ATTR_SERVICE: message.service,
-                    ATTR_UUID: message.uuid,
-                    ATTR_CONFIG: message.config,
-                }
-            )
+        discovery = [
+            {
+                ATTR_ADDON: message.addon,
+                ATTR_SERVICE: message.service,
+                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 = {}
diff --git a/tests/api/test_discovery.py b/tests/api/test_discovery.py
index 106dffabb..304e68f31 100644
--- a/tests/api/test_discovery.py
+++ b/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"] == []
diff --git a/tests/fixtures/discovery.json b/tests/fixtures/discovery.json
new file mode 100644
index 000000000..acf7c320c
--- /dev/null
+++ b/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"
+    }
+  ]
+}