Add warning to custom integrations without version (home-assistant#45919

)

Co-authored-by: Paulus Schoutsen <[email protected]>

homeassistant/loader.py

@@ -26,6 +26,9 @@
     cast,
 )
 
+from awesomeversion import AwesomeVersion
+from awesomeversion.strategy import AwesomeVersionStrategy
+
 from homeassistant.generated.dhcp import DHCP
 from homeassistant.generated.mqtt import MQTT
 from homeassistant.generated.ssdp import SSDP
@@ -52,7 +55,19 @@
     "You are using a custom integration %s which has not "
     "been tested by Home Assistant. This component might "
     "cause stability problems, be sure to disable it if you "
-    "experience issues with Home Assistant."
+    "experience issues with Home Assistant"
+)
+CUSTOM_WARNING_VERSION_MISSING = (
+    "No 'version' key in the manifest file for "
+    "custom integration '%s'. This will not be "
+    "allowed in a future version of Home "
+    "Assistant. Please report this to the "
+    "maintainer of '%s'"
+)
+CUSTOM_WARNING_VERSION_TYPE = (
+    "'%s' is not a valid version for "
+    "custom integration '%s'. "
+    "Please report this to the maintainer of '%s'"
 )
 _UNDEF = object()  # Internal; not helpers.typing.UNDEFINED due to circular dependency
 
@@ -83,6 +98,7 @@ class Manifest(TypedDict, total=False):
     dhcp: List[Dict[str, str]]
     homekit: Dict[str, List[str]]
     is_built_in: bool
+    version: str
     codeowners: List[str]
 
 
@@ -417,6 +433,13 @@ def is_built_in(self) -> bool:
         """Test if package is a built-in integration."""
         return self.pkg_path.startswith(PACKAGE_BUILTIN)
 
+    @property
+    def version(self) -> Optional[AwesomeVersion]:
+        """Return the version of the integration."""
+        if "version" not in self.manifest:
+            return None
+        return AwesomeVersion(self.manifest["version"])
+
     @property
     def all_dependencies(self) -> Set[str]:
         """Return all dependencies including sub-dependencies."""
@@ -513,7 +536,7 @@ async def async_get_integration(hass: "HomeAssistant", domain: str) -> Integrati
     # components to find the integration.
     integration = (await async_get_custom_components(hass)).get(domain)
     if integration is not None:
-        _LOGGER.warning(CUSTOM_WARNING, domain)
+        custom_integration_warning(integration)
         cache[domain] = integration
         event.set()
         return integration
@@ -531,6 +554,7 @@ async def async_get_integration(hass: "HomeAssistant", domain: str) -> Integrati
 
     integration = Integration.resolve_legacy(hass, domain)
     if integration is not None:
+        custom_integration_warning(integration)
         cache[domain] = integration
     else:
         # Remove event from cache.
@@ -605,9 +629,6 @@ def _load_file(
 
             cache[comp_or_platform] = module
 
-            if module.__name__.startswith(PACKAGE_CUSTOM_COMPONENTS):
-                _LOGGER.warning(CUSTOM_WARNING, comp_or_platform)
-
             return module
 
         except ImportError as err:
@@ -756,3 +777,35 @@ def _lookup_path(hass: "HomeAssistant") -> List[str]:
     if hass.config.safe_mode:
         return [PACKAGE_BUILTIN]
     return [PACKAGE_CUSTOM_COMPONENTS, PACKAGE_BUILTIN]
+
+
+def validate_custom_integration_version(version: str) -> bool:
+    """Validate the version of custom integrations."""
+    return AwesomeVersion(version).strategy in (
+        AwesomeVersionStrategy.CALVER,
+        AwesomeVersionStrategy.SEMVER,
+        AwesomeVersionStrategy.SIMPLEVER,
+        AwesomeVersionStrategy.BUILDVER,
+        AwesomeVersionStrategy.PEP440,
+    )
+
+
+def custom_integration_warning(integration: Integration) -> None:
+    """Create logs for custom integrations."""
+    if not integration.pkg_path.startswith(PACKAGE_CUSTOM_COMPONENTS):
+        return None
+
+    _LOGGER.warning(CUSTOM_WARNING, integration.domain)
+
+    if integration.manifest.get("version") is None:
+        _LOGGER.warning(
+            CUSTOM_WARNING_VERSION_MISSING, integration.domain, integration.domain
+        )
+    else:
+        if not validate_custom_integration_version(integration.manifest["version"]):
+            _LOGGER.warning(
+                CUSTOM_WARNING_VERSION_TYPE,
+                integration.domain,
+                integration.manifest["version"],
+                integration.domain,
+            )

homeassistant/package_constraints.txt

@@ -5,6 +5,7 @@ aiohttp_cors==0.7.0
 astral==1.10.1
 async_timeout==3.0.1
 attrs==19.3.0
+awesomeversion==21.2.0
 bcrypt==3.1.7
 certifi>=2020.12.5
 ciso8601==2.1.3

requirements.txt

@@ -5,6 +5,7 @@ aiohttp==3.7.3
 astral==1.10.1
 async_timeout==3.0.1
 attrs==19.3.0
+awesomeversion==21.2.0
 bcrypt==3.1.7
 certifi>=2020.12.5
 ciso8601==2.1.3

requirements_test.txt

@@ -13,7 +13,6 @@ pre-commit==2.10.0
 pylint==2.6.0
 astroid==2.4.2
 pipdeptree==1.0.0
-awesomeversion==21.2.0
 pylint-strict-informational==0.1
 pytest-aiohttp==0.3.0
 pytest-cov==2.10.1

script/bootstrap

@@ -16,4 +16,4 @@ fi
 
 echo "Installing development dependencies..."
 python3 -m pip install wheel --constraint homeassistant/package_constraints.txt
-python3 -m pip install tox colorlog pre-commit $(grep mypy requirements_test.txt) $(grep stdlib-list requirements_test.txt) $(grep tqdm requirements_test.txt) $(grep pipdeptree requirements_test.txt) $(grep awesomeversion requirements_test.txt) --constraint homeassistant/package_constraints.txt
+python3 -m pip install tox colorlog pre-commit $(grep mypy requirements_test.txt) $(grep stdlib-list requirements_test.txt) $(grep tqdm requirements_test.txt) $(grep pipdeptree requirements_test.txt) $(grep awesomeversion requirements.txt) --constraint homeassistant/package_constraints.txt

script/hassfest/manifest.py

@@ -2,11 +2,11 @@
 from typing import Dict
 from urllib.parse import urlparse
 
-from awesomeversion import AwesomeVersion
-from awesomeversion.strategy import AwesomeVersionStrategy
 import voluptuous as vol
 from voluptuous.humanize import humanize_error
 
+from homeassistant.loader import validate_custom_integration_version
+
 from .model import Integration
 
 DOCUMENTATION_URL_SCHEMA = "https"
@@ -53,16 +53,9 @@ def verify_uppercase(value: str):
 
 def verify_version(value: str):
     """Verify the version."""
-    version = AwesomeVersion(value)
-    if version.strategy not in [
-        AwesomeVersionStrategy.CALVER,
-        AwesomeVersionStrategy.SEMVER,
-        AwesomeVersionStrategy.SIMPLEVER,
-        AwesomeVersionStrategy.BUILDVER,
-        AwesomeVersionStrategy.PEP440,
-    ]:
+    if not validate_custom_integration_version(value):
         raise vol.Invalid(
-            f"'{version}' is not a valid version. This will cause a future version of Home Assistant to block this integration.",
+            f"'{value}' is not a valid version. This will cause a future version of Home Assistant to block this integration.",
         )
     return value
 

setup.py

@@ -36,6 +36,7 @@
     "astral==1.10.1",
     "async_timeout==3.0.1",
     "attrs==19.3.0",
+    "awesomeversion==21.2.0",
     "bcrypt==3.1.7",
     "certifi>=2020.12.5",
     "ciso8601==2.1.3",

tests/hassfest/test_version.py

@@ -0,0 +1,47 @@
+"""Tests for hassfest version."""
+import pytest
+import voluptuous as vol
+
+from script.hassfest.manifest import (
+    CUSTOM_INTEGRATION_MANIFEST_SCHEMA,
+    validate_version,
+)
+from script.hassfest.model import Integration
+
+
+@pytest.fixture
+def integration():
+    """Fixture for hassfest integration model."""
+    integration = Integration("")
+    integration.manifest = {
+        "domain": "test",
+        "documentation": "https://example.com",
+        "name": "test",
+        "codeowners": ["@awesome"],
+    }
+    return integration
+
+
+def test_validate_version_no_key(integration: Integration):
+    """Test validate version with no key."""
+    validate_version(integration)
+    assert (
+        "No 'version' key in the manifest file. This will cause a future version of Home Assistant to block this integration."
+        in [x.error for x in integration.warnings]
+    )
+
+
+def test_validate_custom_integration_manifest(integration: Integration):
+    """Test validate custom integration manifest."""
+
+    with pytest.raises(vol.Invalid):
+        integration.manifest["version"] = "lorem_ipsum"
+        CUSTOM_INTEGRATION_MANIFEST_SCHEMA(integration.manifest)
+
+    with pytest.raises(vol.Invalid):
+        integration.manifest["version"] = None
+        CUSTOM_INTEGRATION_MANIFEST_SCHEMA(integration.manifest)
+
+    integration.manifest["version"] = "1"
+    schema = CUSTOM_INTEGRATION_MANIFEST_SCHEMA(integration.manifest)
+    assert schema["version"] == "1"

tests/test_loader.py

@@ -130,13 +130,69 @@ async def test_custom_component_name(hass):
 
 async def test_log_warning_custom_component(hass, caplog):
     """Test that we log a warning when loading a custom component."""
-    hass.components.test_standalone
+    await loader.async_get_integration(hass, "test_standalone")
     assert "You are using a custom integration test_standalone" in caplog.text
 
     await loader.async_get_integration(hass, "test")
     assert "You are using a custom integration test " in caplog.text
 
 
+async def test_custom_integration_missing_version(hass, caplog):
+    """Test that we log a warning when custom integrations are missing a version."""
+    test_integration_1 = loader.Integration(
+        hass, "custom_components.test1", None, {"domain": "test1"}
+    )
+    test_integration_2 = loader.Integration(
+        hass,
+        "custom_components.test2",
+        None,
+        loader.manifest_from_legacy_module("test2", "custom_components.test2"),
+    )
+
+    with patch("homeassistant.loader.async_get_custom_components") as mock_get:
+        mock_get.return_value = {
+            "test1": test_integration_1,
+            "test2": test_integration_2,
+        }
+
+        await loader.async_get_integration(hass, "test1")
+        assert (
+            "No 'version' key in the manifest file for custom integration 'test1'."
+            in caplog.text
+        )
+
+        await loader.async_get_integration(hass, "test2")
+        assert (
+            "No 'version' key in the manifest file for custom integration 'test2'."
+            in caplog.text
+        )
+
+
+async def test_no_version_warning_for_none_custom_integrations(hass, caplog):
+    """Test that we do not log a warning when core integrations are missing a version."""
+    await loader.async_get_integration(hass, "hue")
+    assert (
+        "No 'version' key in the manifest file for custom integration 'hue'."
+        not in caplog.text
+    )
+
+
+async def test_custom_integration_version_not_valid(hass, caplog):
+    """Test that we log a warning when custom integrations have a invalid version."""
+    test_integration = loader.Integration(
+        hass, "custom_components.test", None, {"domain": "test", "version": "test"}
+    )
+
+    with patch("homeassistant.loader.async_get_custom_components") as mock_get:
+        mock_get.return_value = {"test": test_integration}
+
+        await loader.async_get_integration(hass, "test")
+        assert (
+            "'test' is not a valid version for custom integration 'test'."
+            in caplog.text
+        )
+
+
 async def test_get_integration(hass):
     """Test resolving integration."""
     integration = await loader.async_get_integration(hass, "hue")
@@ -154,7 +210,6 @@ async def test_get_integration_legacy(hass):
 async def test_get_integration_custom_component(hass, enable_custom_integrations):
     """Test resolving integration."""
     integration = await loader.async_get_integration(hass, "test_package")
-    print(integration)
     assert integration.get_component().DOMAIN == "test_package"
     assert integration.name == "Test Package"
 
@@ -189,6 +244,7 @@ def test_integration_properties(hass):
                 {"manufacturer": "Signify", "modelName": "Philips hue bridge 2015"},
             ],
             "mqtt": ["hue/discovery"],
+            "version": "1.0.0",
         },
     )
     assert integration.name == "Philips Hue"
@@ -215,6 +271,7 @@ def test_integration_properties(hass):
     assert integration.dependencies == ["test-dep"]
     assert integration.requirements == ["test-req==1.0.0"]
     assert integration.is_built_in is True
+    assert integration.version == "1.0.0"
 
     integration = loader.Integration(
         hass,
@@ -233,6 +290,7 @@ def test_integration_properties(hass):
     assert integration.dhcp is None
     assert integration.ssdp is None
     assert integration.mqtt is None
+    assert integration.version is None
 
     integration = loader.Integration(
         hass,