From 1ab215699860a5114cb4a7690dc894b76cb02161 Mon Sep 17 00:00:00 2001 From: Jason Lawrence Date: Fri, 21 Feb 2020 11:41:47 -0600 Subject: [PATCH 01/15] Cert Expiry refactor --- .coveragerc | 4 +- .../components/cert_expiry/config_flow.py | 76 ++++---- homeassistant/components/cert_expiry/const.py | 1 - .../components/cert_expiry/errors.py | 14 ++ .../components/cert_expiry/helper.py | 29 ++- .../components/cert_expiry/sensor.py | 110 +++++++----- .../components/cert_expiry/strings.json | 7 +- .../cert_expiry/test_config_flow.py | 169 +++++++++--------- 8 files changed, 236 insertions(+), 174 deletions(-) create mode 100644 homeassistant/components/cert_expiry/errors.py diff --git a/.coveragerc b/.coveragerc index 6dcd20f6ad6eb1..75153dd4e08cd8 100644 --- a/.coveragerc +++ b/.coveragerc @@ -107,8 +107,10 @@ omit = homeassistant/components/canary/alarm_control_panel.py homeassistant/components/canary/camera.py homeassistant/components/cast/* - homeassistant/components/cert_expiry/sensor.py + homeassistant/components/cert_expiry/__init__.py + homeassistant/components/cert_expiry/errors.py homeassistant/components/cert_expiry/helper.py + homeassistant/components/cert_expiry/sensor.py homeassistant/components/channels/* homeassistant/components/cisco_ios/device_tracker.py homeassistant/components/cisco_mobility_express/device_tracker.py diff --git a/homeassistant/components/cert_expiry/config_flow.py b/homeassistant/components/cert_expiry/config_flow.py index f3bd2f07d63689..4b884349484cbc 100644 --- a/homeassistant/components/cert_expiry/config_flow.py +++ b/homeassistant/components/cert_expiry/config_flow.py @@ -1,16 +1,16 @@ """Config flow for the Cert Expiry platform.""" import logging import socket -import ssl import voluptuous as vol from homeassistant import config_entries -from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT +from homeassistant.const import CONF_HOST, CONF_PORT from homeassistant.core import HomeAssistant, callback -from .const import DEFAULT_NAME, DEFAULT_PORT, DOMAIN -from .helper import get_cert +from .const import DEFAULT_PORT, DOMAIN +from .errors import TemporaryFailure, ValidationFailure +from .helper import get_cert_time_to_expiry _LOGGER = logging.getLogger(__name__) @@ -43,50 +43,51 @@ def _prt_in_configuration_exists(self, user_input) -> bool: return False async def _test_connection(self, user_input=None): - """Test connection to the server and try to get the certtificate.""" - host = user_input[CONF_HOST] + """Test connection to the server and try to get the certificate.""" try: - await self.hass.async_add_executor_job( - get_cert, host, user_input.get(CONF_PORT, DEFAULT_PORT) + await get_cert_time_to_expiry( + self.hass, + user_input[CONF_HOST], + user_input.get(CONF_PORT, DEFAULT_PORT), ) return True - except socket.gaierror: - _LOGGER.error("Host cannot be resolved: %s", host) - self._errors[CONF_HOST] = "resolve_failed" - except socket.timeout: - _LOGGER.error("Timed out connecting to %s", host) - self._errors[CONF_HOST] = "connection_timeout" - except ssl.CertificateError as err: - if "doesn't match" in err.args[0]: - _LOGGER.error("Certificate does not match host: %s", host) - self._errors[CONF_HOST] = "wrong_host" - else: - _LOGGER.error("Certificate could not be validated: %s", host) - self._errors[CONF_HOST] = "certificate_error" - except ssl.SSLError: - _LOGGER.error("Certificate could not be validated: %s", host) - self._errors[CONF_HOST] = "certificate_error" + except TemporaryFailure as err: + cause = type(err.__cause__) + if cause is socket.gaierror: + self._errors[CONF_HOST] = "resolve_failed" + elif cause is socket.timeout: + self._errors[CONF_HOST] = "connection_timeout" + elif cause is ConnectionRefusedError: + self._errors[CONF_HOST] = "connection_refused" + except ValidationFailure: + return True return False async def async_step_user(self, user_input=None): """Step when user initializes a integration.""" self._errors = {} if user_input is not None: - # set some defaults in case we need to return to the form if self._prt_in_configuration_exists(user_input): - self._errors[CONF_HOST] = "host_port_exists" - else: - if await self._test_connection(user_input): - return self.async_create_entry( - title=user_input.get(CONF_NAME, DEFAULT_NAME), - data={ - CONF_HOST: user_input[CONF_HOST], - CONF_PORT: user_input.get(CONF_PORT, DEFAULT_PORT), - }, - ) + return self.async_abort(reason="host_port_exists") + + if await self._test_connection(user_input): + host = user_input[CONF_HOST] + port = user_input.get(CONF_PORT, DEFAULT_PORT) + title = host + (f":{port}" if port != DEFAULT_PORT else "") + return self.async_create_entry( + title=title, + data={ + CONF_HOST: user_input[CONF_HOST], + CONF_PORT: user_input.get(CONF_PORT, DEFAULT_PORT), + }, + ) + if ( # pylint: disable=no-member + self.context["source"] == config_entries.SOURCE_IMPORT + ): + _LOGGER.error("Config import failed for %s", user_input[CONF_HOST]) + return self.async_abort(reason="import_failed") else: user_input = {} - user_input[CONF_NAME] = DEFAULT_NAME user_input[CONF_HOST] = "" user_input[CONF_PORT] = DEFAULT_PORT @@ -94,9 +95,6 @@ async def async_step_user(self, user_input=None): step_id="user", data_schema=vol.Schema( { - vol.Required( - CONF_NAME, default=user_input.get(CONF_NAME, DEFAULT_NAME) - ): str, vol.Required(CONF_HOST, default=user_input[CONF_HOST]): str, vol.Required( CONF_PORT, default=user_input.get(CONF_PORT, DEFAULT_PORT) diff --git a/homeassistant/components/cert_expiry/const.py b/homeassistant/components/cert_expiry/const.py index 4129781f2a0fb2..00d5ac9e923f6f 100644 --- a/homeassistant/components/cert_expiry/const.py +++ b/homeassistant/components/cert_expiry/const.py @@ -1,6 +1,5 @@ """Const for Cert Expiry.""" DOMAIN = "cert_expiry" -DEFAULT_NAME = "SSL Certificate Expiry" DEFAULT_PORT = 443 TIMEOUT = 10.0 diff --git a/homeassistant/components/cert_expiry/errors.py b/homeassistant/components/cert_expiry/errors.py new file mode 100644 index 00000000000000..dadb09f395d450 --- /dev/null +++ b/homeassistant/components/cert_expiry/errors.py @@ -0,0 +1,14 @@ +"""Errors for the cert_expiry integration.""" +from homeassistant.exceptions import HomeAssistantError + + +class CertExpiryException(HomeAssistantError): + """Base class for cert_expiry exceptions.""" + + +class TemporaryFailure(CertExpiryException): + """Temporary failure has occurred.""" + + +class ValidationFailure(CertExpiryException): + """Certificate validation failure has occurred.""" diff --git a/homeassistant/components/cert_expiry/helper.py b/homeassistant/components/cert_expiry/helper.py index cd49588ec89f31..9044b82a83e2b1 100644 --- a/homeassistant/components/cert_expiry/helper.py +++ b/homeassistant/components/cert_expiry/helper.py @@ -1,12 +1,14 @@ """Helper functions for the Cert Expiry platform.""" +from datetime import datetime import socket import ssl from .const import TIMEOUT +from .errors import TemporaryFailure, ValidationFailure def get_cert(host, port): - """Get the ssl certificate for the host and port combination.""" + """Get the certificate for the host and port combination.""" ctx = ssl.create_default_context() address = (host, port) with socket.create_connection(address, timeout=TIMEOUT) as sock: @@ -14,3 +16,28 @@ def get_cert(host, port): # pylint disable: https://github.com/PyCQA/pylint/issues/3166 cert = ssock.getpeercert() # pylint: disable=no-member return cert + + +async def get_cert_time_to_expiry(hass, hostname, port): + """Return the certificate's time to expiry in days.""" + try: + cert = await hass.async_add_executor_job(get_cert, hostname, port) + except socket.gaierror as err: + raise TemporaryFailure(f"Cannot resolve hostname: {hostname}") from err + except socket.timeout as err: + raise TemporaryFailure( + f"Connection timeout with server: {hostname}:{port}" + ) from err + except ConnectionRefusedError as err: + raise TemporaryFailure( + f"Connection refused by server: {hostname}:{port}" + ) from err + except ssl.CertificateError as err: + raise ValidationFailure(err.verify_message) + except ssl.SSLError as err: + raise ValidationFailure(err.args[0]) + + ts_seconds = ssl.cert_time_to_seconds(cert["notAfter"]) + timestamp = datetime.fromtimestamp(ts_seconds) + expiry = timestamp - datetime.today() + return expiry.days diff --git a/homeassistant/components/cert_expiry/sensor.py b/homeassistant/components/cert_expiry/sensor.py index b4437ca5834fd4..acd67874234f4a 100644 --- a/homeassistant/components/cert_expiry/sensor.py +++ b/homeassistant/components/cert_expiry/sensor.py @@ -1,8 +1,6 @@ """Counter for the days until an HTTPS (TLS) certificate will expire.""" -from datetime import datetime, timedelta +from datetime import timedelta import logging -import socket -import ssl import voluptuous as vol @@ -16,11 +14,15 @@ TIME_DAYS, ) from homeassistant.core import callback +from homeassistant.exceptions import PlatformNotReady import homeassistant.helpers.config_validation as cv from homeassistant.helpers.entity import Entity +from homeassistant.helpers.event import async_track_point_in_utc_time +from homeassistant.util import dt as dt_util -from .const import DEFAULT_NAME, DEFAULT_PORT, DOMAIN -from .helper import get_cert +from .const import DEFAULT_PORT, DOMAIN +from .errors import TemporaryFailure, ValidationFailure +from .helper import get_cert_time_to_expiry _LOGGER = logging.getLogger(__name__) @@ -29,7 +31,7 @@ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend( { vol.Required(CONF_HOST): cv.string, - vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string, + vol.Optional(CONF_NAME): cv.string, # Deprecated vol.Optional(CONF_PORT, default=DEFAULT_PORT): cv.port, } ) @@ -38,25 +40,42 @@ async def async_setup_platform(hass, config, async_add_entities, discovery_info=None): """Set up certificate expiry sensor.""" + @callback + def schedule_import(_): + """Schedule delayed import after HA is fully started.""" + import_time = dt_util.utcnow() + timedelta(seconds=30) + async_track_point_in_utc_time(hass, do_import, import_time) + @callback def do_import(_): - """Process YAML import after HA is fully started.""" + """Process YAML import.""" hass.async_create_task( hass.config_entries.flow.async_init( DOMAIN, context={"source": SOURCE_IMPORT}, data=dict(config) ) ) - # Delay to avoid validation during setup in case we're checking our own cert. - hass.bus.async_listen_once(EVENT_HOMEASSISTANT_START, do_import) + hass.bus.async_listen_once(EVENT_HOMEASSISTANT_START, schedule_import) async def async_setup_entry(hass, entry, async_add_entities): """Add cert-expiry entry.""" + days = 0 + error = None + hostname = entry.data[CONF_HOST] + port = entry.data[CONF_PORT] + + try: + days = await get_cert_time_to_expiry(hass, hostname, port) + except TemporaryFailure as err: + _LOGGER.error(err) + raise PlatformNotReady + except ValidationFailure as err: + error = err + pass + async_add_entities( - [SSLCertificate(entry.title, entry.data[CONF_HOST], entry.data[CONF_PORT])], - False, - # Don't update in case we're checking our own cert. + [SSLCertificate(hass, entry.title, hostname, port, days, error)], False, ) return True @@ -64,14 +83,21 @@ async def async_setup_entry(hass, entry, async_add_entities): class SSLCertificate(Entity): """Implementation of the certificate expiry sensor.""" - def __init__(self, sensor_name, server_name, server_port): + def __init__(self, hass, sensor_name, server_name, server_port, days, error): """Initialize the sensor.""" + self.hass = hass self.server_name = server_name self.server_port = server_port - self._name = sensor_name - self._state = None + display_port = f":{server_port}" if server_port != DEFAULT_PORT else "" + self._name = f"Cert Expiry ({self.server_name}{display_port})" self._available = False + self._error = error + self._state = None self._valid = False + if days is not None: + self._state = days + self._available = True + self._valid = True @property def name(self): @@ -103,50 +129,42 @@ def available(self): """Return the availability of the sensor.""" return self._available - async def async_added_to_hass(self): - """Once the entity is added we should update to get the initial data loaded.""" - - @callback - def do_update(_): - """Run the update method when the start event was fired.""" - self.async_schedule_update_ha_state(True) - - if self.hass.is_running: - self.async_schedule_update_ha_state(True) - else: - # Delay until HA is fully started in case we're checking our own cert. - self.hass.bus.async_listen_once(EVENT_HOMEASSISTANT_START, do_update) - - def update(self): + async def async_update(self): """Fetch the certificate information.""" try: - cert = get_cert(self.server_name, self.server_port) - except socket.gaierror: - _LOGGER.error("Cannot resolve hostname: %s", self.server_name) - self._available = False - self._valid = False - return - except socket.timeout: - _LOGGER.error("Connection timeout with server: %s", self.server_name) + days_to_expiry = await get_cert_time_to_expiry( + self.hass, self.server_name, self.server_port + ) + except TemporaryFailure as err: + _LOGGER.error(err) self._available = False + self._error = err self._valid = False return - except (ssl.CertificateError, ssl.SSLError): + except ValidationFailure as err: + _LOGGER.error( + "Certificate validation error: %s [%s]", self.server_name, err + ) self._available = True + self._error = err self._state = 0 self._valid = False return + except Exception: # pylint: disable=broad-except + _LOGGER.exception( + "Unknown error checking %s:%s", self.server_name, self.server_port + ) + self._available = False + self._error = "Unknown" + self._valid = False + return - ts_seconds = ssl.cert_time_to_seconds(cert["notAfter"]) - timestamp = datetime.fromtimestamp(ts_seconds) - expiry = timestamp - datetime.today() self._available = True - self._state = expiry.days + self._error = None + self._state = days_to_expiry self._valid = True @property def device_state_attributes(self): """Return additional sensor state attributes.""" - attr = {"is_valid": self._valid} - - return attr + return {"is_valid": self._valid, "error": str(self._error)} diff --git a/homeassistant/components/cert_expiry/strings.json b/homeassistant/components/cert_expiry/strings.json index e5e670d214fc4a..286239fd5a5d49 100644 --- a/homeassistant/components/cert_expiry/strings.json +++ b/homeassistant/components/cert_expiry/strings.json @@ -12,14 +12,13 @@ } }, "error": { - "host_port_exists": "This host and port combination is already configured", "resolve_failed": "This host can not be resolved", "connection_timeout": "Timeout when connecting to this host", - "certificate_error": "Certificate could not be validated", - "wrong_host": "Certificate does not match hostname" + "connection_refused": "Connection refused when connecting to host" }, "abort": { - "host_port_exists": "This host and port combination is already configured" + "host_port_exists": "This host and port combination is already configured", + "import_failed": "Import from config failed" } } } diff --git a/tests/components/cert_expiry/test_config_flow.py b/tests/components/cert_expiry/test_config_flow.py index 71005672fdb66a..fcac1c0747730e 100644 --- a/tests/components/cert_expiry/test_config_flow.py +++ b/tests/components/cert_expiry/test_config_flow.py @@ -3,38 +3,33 @@ import ssl from unittest.mock import patch -import pytest +from asynctest import patch as asyncpatch from homeassistant import data_entry_flow from homeassistant.components.cert_expiry import config_flow -from homeassistant.components.cert_expiry.const import DEFAULT_NAME, DEFAULT_PORT +from homeassistant.components.cert_expiry.const import DEFAULT_PORT +from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_USER from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT -from tests.common import MockConfigEntry, mock_coro +from tests.common import MockConfigEntry -NAME = "Cert Expiry test 1 2 3" PORT = 443 HOST = "example.com" +GET_CERT_METHOD = "homeassistant.components.cert_expiry.helper.get_cert" +GET_CERT_TIME_METHOD = ( + "homeassistant.components.cert_expiry.config_flow.get_cert_time_to_expiry" +) -@pytest.fixture(name="test_connect") -def mock_controller(): - """Mock a successful _prt_in_configuration_exists.""" - with patch( - "homeassistant.components.cert_expiry.config_flow.CertexpiryConfigFlow._test_connection", - side_effect=lambda *_: mock_coro(True), - ): - yield - - -def init_config_flow(hass): +def init_config_flow(hass, source=SOURCE_USER): """Init a configuration flow.""" flow = config_flow.CertexpiryConfigFlow() flow.hass = hass + flow.context = {"source": source} return flow -async def test_user(hass, test_connect): +async def test_user(hass): """Test user config.""" flow = init_config_flow(hass) @@ -42,113 +37,123 @@ async def test_user(hass, test_connect): assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["step_id"] == "user" - # tets with all provided - result = await flow.async_step_user( - {CONF_NAME: NAME, CONF_HOST: HOST, CONF_PORT: PORT} - ) + # test with all provided + with asyncpatch(GET_CERT_TIME_METHOD, return_value=1): + result = await flow.async_step_user({CONF_HOST: HOST, CONF_PORT: PORT}) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY - assert result["title"] == NAME + assert result["title"] == HOST assert result["data"][CONF_HOST] == HOST assert result["data"][CONF_PORT] == PORT -async def test_import(hass, test_connect): - """Test import step.""" +async def test_user_with_bad_cert(hass): + """Test user config.""" flow = init_config_flow(hass) - # import with only host - result = await flow.async_step_import({CONF_HOST: HOST}) + result = await flow.async_step_user() + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + + # import with certification validation error + with patch(GET_CERT_METHOD, side_effect=ssl.SSLError("some error")): + result = await flow.async_step_user({CONF_HOST: HOST, CONF_PORT: PORT}) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY - assert result["title"] == DEFAULT_NAME + assert result["title"] == HOST assert result["data"][CONF_HOST] == HOST - assert result["data"][CONF_PORT] == DEFAULT_PORT + assert result["data"][CONF_PORT] == PORT - # import with host and name - result = await flow.async_step_import({CONF_HOST: HOST, CONF_NAME: NAME}) + +async def test_import(hass): + """Test import step.""" + flow = init_config_flow(hass, source=SOURCE_IMPORT) + + # import with only host + with asyncpatch(GET_CERT_TIME_METHOD, return_value=1): + result = await flow.async_step_import({CONF_HOST: HOST}) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY - assert result["title"] == NAME + assert result["title"] == HOST assert result["data"][CONF_HOST] == HOST assert result["data"][CONF_PORT] == DEFAULT_PORT - # improt with host and port - result = await flow.async_step_import({CONF_HOST: HOST, CONF_PORT: PORT}) + # import with host and port + with asyncpatch(GET_CERT_TIME_METHOD, return_value=1): + result = await flow.async_step_import({CONF_HOST: HOST, CONF_PORT: PORT}) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY - assert result["title"] == DEFAULT_NAME + assert result["title"] == HOST assert result["data"][CONF_HOST] == HOST assert result["data"][CONF_PORT] == PORT - # import with all - result = await flow.async_step_import( - {CONF_HOST: HOST, CONF_PORT: PORT, CONF_NAME: NAME} - ) + # import with host and non-default port + with asyncpatch(GET_CERT_TIME_METHOD, return_value=1): + result = await flow.async_step_import({CONF_HOST: HOST, CONF_PORT: 888}) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY - assert result["title"] == NAME + assert result["title"] == f"{HOST}:888" + assert result["data"][CONF_HOST] == HOST + assert result["data"][CONF_PORT] == 888 + + # import legacy config with name + with asyncpatch(GET_CERT_TIME_METHOD, return_value=1): + result = await flow.async_step_import( + {CONF_NAME: "legacy", CONF_HOST: HOST, CONF_PORT: PORT} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["title"] == HOST assert result["data"][CONF_HOST] == HOST assert result["data"][CONF_PORT] == PORT -async def test_abort_if_already_setup(hass, test_connect): +async def test_bad_import(hass): + """Test import step.""" + flow = init_config_flow(hass, source=SOURCE_IMPORT) + + with patch(GET_CERT_METHOD, side_effect=ConnectionRefusedError()): + result = await flow.async_step_import({CONF_HOST: HOST}) + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "import_failed" + + +async def test_abort_if_already_setup(hass): """Test we abort if the cert is already setup.""" - flow = init_config_flow(hass) MockConfigEntry( - domain="cert_expiry", - data={CONF_PORT: DEFAULT_PORT, CONF_NAME: NAME, CONF_HOST: HOST}, + domain="cert_expiry", data={CONF_PORT: DEFAULT_PORT, CONF_HOST: HOST}, ).add_to_hass(hass) + # Test with import source + flow = init_config_flow(hass, source=SOURCE_IMPORT) + # Should fail, same HOST and PORT (default) - result = await flow.async_step_import( - {CONF_HOST: HOST, CONF_NAME: NAME, CONF_PORT: DEFAULT_PORT} - ) + result = await flow.async_step_import({CONF_HOST: HOST, CONF_PORT: DEFAULT_PORT}) assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT assert result["reason"] == "host_port_exists" - # Should be the same HOST and PORT (default) - result = await flow.async_step_user( - {CONF_HOST: HOST, CONF_NAME: NAME, CONF_PORT: DEFAULT_PORT} - ) + # Test with user source + flow = init_config_flow(hass) + + result = await flow.async_step_user() assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["errors"] == {CONF_HOST: "host_port_exists"} + assert result["step_id"] == "user" - # SHOULD pass, same Host diff PORT - result = await flow.async_step_import( - {CONF_HOST: HOST, CONF_NAME: NAME, CONF_PORT: 888} - ) - assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY - assert result["title"] == NAME - assert result["data"][CONF_HOST] == HOST - assert result["data"][CONF_PORT] == 888 + # Should fail, same HOST and PORT (default) + result = await flow.async_step_user({CONF_HOST: HOST, CONF_PORT: DEFAULT_PORT}) + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "host_port_exists" async def test_abort_on_socket_failed(hass): """Test we abort of we have errors during socket creation.""" flow = init_config_flow(hass) - with patch("socket.create_connection", side_effect=socket.gaierror()): - result = await flow.async_step_user({CONF_HOST: HOST}) - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["errors"] == {CONF_HOST: "resolve_failed"} - - with patch("socket.create_connection", side_effect=socket.timeout()): + with patch(GET_CERT_METHOD, side_effect=socket.gaierror()): result = await flow.async_step_user({CONF_HOST: HOST}) - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["errors"] == {CONF_HOST: "connection_timeout"} - - with patch( - "socket.create_connection", - side_effect=ssl.CertificateError(f"{HOST} doesn't match somethingelse.com"), - ): - result = await flow.async_step_user({CONF_HOST: HOST}) - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["errors"] == {CONF_HOST: "wrong_host"} + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["errors"] == {CONF_HOST: "resolve_failed"} - with patch( - "socket.create_connection", side_effect=ssl.CertificateError("different error") - ): + with patch(GET_CERT_METHOD, side_effect=socket.timeout()): result = await flow.async_step_user({CONF_HOST: HOST}) - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["errors"] == {CONF_HOST: "certificate_error"} + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["errors"] == {CONF_HOST: "connection_timeout"} - with patch("socket.create_connection", side_effect=ssl.SSLError()): + with patch(GET_CERT_METHOD, side_effect=ConnectionRefusedError): result = await flow.async_step_user({CONF_HOST: HOST}) - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["errors"] == {CONF_HOST: "certificate_error"} + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["errors"] == {CONF_HOST: "connection_refused"} From 602615196631804667c6909d705c8d4a9810942d Mon Sep 17 00:00:00 2001 From: Jason Lawrence Date: Sat, 22 Feb 2020 22:00:05 -0600 Subject: [PATCH 02/15] Unused parameter --- homeassistant/components/cert_expiry/sensor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/cert_expiry/sensor.py b/homeassistant/components/cert_expiry/sensor.py index acd67874234f4a..44a2bbdee06f38 100644 --- a/homeassistant/components/cert_expiry/sensor.py +++ b/homeassistant/components/cert_expiry/sensor.py @@ -75,7 +75,7 @@ async def async_setup_entry(hass, entry, async_add_entities): pass async_add_entities( - [SSLCertificate(hass, entry.title, hostname, port, days, error)], False, + [SSLCertificate(hass, hostname, port, days, error)], False, ) return True @@ -83,7 +83,7 @@ async def async_setup_entry(hass, entry, async_add_entities): class SSLCertificate(Entity): """Implementation of the certificate expiry sensor.""" - def __init__(self, hass, sensor_name, server_name, server_port, days, error): + def __init__(self, hass, server_name, server_port, days, error): """Initialize the sensor.""" self.hass = hass self.server_name = server_name From 051688a48006db2ac6514ed058aa09348709d902 Mon Sep 17 00:00:00 2001 From: Jason Lawrence Date: Sat, 22 Feb 2020 22:00:37 -0600 Subject: [PATCH 03/15] Reduce delay --- homeassistant/components/cert_expiry/sensor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/cert_expiry/sensor.py b/homeassistant/components/cert_expiry/sensor.py index 44a2bbdee06f38..4cc43533cf483e 100644 --- a/homeassistant/components/cert_expiry/sensor.py +++ b/homeassistant/components/cert_expiry/sensor.py @@ -43,7 +43,7 @@ async def async_setup_platform(hass, config, async_add_entities, discovery_info= @callback def schedule_import(_): """Schedule delayed import after HA is fully started.""" - import_time = dt_util.utcnow() + timedelta(seconds=30) + import_time = dt_util.utcnow() + timedelta(seconds=10) async_track_point_in_utc_time(hass, do_import, import_time) @callback From 60cf3fb3afde901dc22c0f93a5dde80a5a8be19f Mon Sep 17 00:00:00 2001 From: Jason Lawrence Date: Sat, 22 Feb 2020 22:02:05 -0600 Subject: [PATCH 04/15] Deprecate 'name' config --- homeassistant/components/cert_expiry/sensor.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/homeassistant/components/cert_expiry/sensor.py b/homeassistant/components/cert_expiry/sensor.py index 4cc43533cf483e..b0c393cf820a42 100644 --- a/homeassistant/components/cert_expiry/sensor.py +++ b/homeassistant/components/cert_expiry/sensor.py @@ -28,12 +28,15 @@ SCAN_INTERVAL = timedelta(hours=12) -PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend( - { - vol.Required(CONF_HOST): cv.string, - vol.Optional(CONF_NAME): cv.string, # Deprecated - vol.Optional(CONF_PORT, default=DEFAULT_PORT): cv.port, - } +PLATFORM_SCHEMA = vol.All( + cv.deprecated(CONF_NAME, invalidation_version="0.109"), + PLATFORM_SCHEMA.extend( + { + vol.Required(CONF_HOST): cv.string, + vol.Optional(CONF_NAME): cv.string, + vol.Optional(CONF_PORT, default=DEFAULT_PORT): cv.port, + } + ), ) From 6ea4b81045f0c65e95434a694a9d6c33304cd439 Mon Sep 17 00:00:00 2001 From: Jason Lawrence Date: Sun, 23 Feb 2020 23:40:33 -0600 Subject: [PATCH 05/15] Use config entry unique_id --- .../components/cert_expiry/config_flow.py | 30 ++++--------------- .../components/cert_expiry/sensor.py | 3 ++ 2 files changed, 8 insertions(+), 25 deletions(-) diff --git a/homeassistant/components/cert_expiry/config_flow.py b/homeassistant/components/cert_expiry/config_flow.py index 4b884349484cbc..f40f9970cd88b0 100644 --- a/homeassistant/components/cert_expiry/config_flow.py +++ b/homeassistant/components/cert_expiry/config_flow.py @@ -6,24 +6,14 @@ from homeassistant import config_entries from homeassistant.const import CONF_HOST, CONF_PORT -from homeassistant.core import HomeAssistant, callback -from .const import DEFAULT_PORT, DOMAIN +from .const import DEFAULT_PORT, DOMAIN # pylint: disable=unused-import from .errors import TemporaryFailure, ValidationFailure from .helper import get_cert_time_to_expiry _LOGGER = logging.getLogger(__name__) -@callback -def certexpiry_entries(hass: HomeAssistant): - """Return the host,port tuples for the domain.""" - return set( - (entry.data[CONF_HOST], entry.data[CONF_PORT]) - for entry in hass.config_entries.async_entries(DOMAIN) - ) - - class CertexpiryConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Handle a config flow.""" @@ -34,14 +24,6 @@ def __init__(self) -> None: """Initialize the config flow.""" self._errors = {} - def _prt_in_configuration_exists(self, user_input) -> bool: - """Return True if host, port combination exists in configuration.""" - host = user_input[CONF_HOST] - port = user_input.get(CONF_PORT, DEFAULT_PORT) - if (host, port) in certexpiry_entries(self.hass): - return True - return False - async def _test_connection(self, user_input=None): """Test connection to the server and try to get the certificate.""" try: @@ -67,12 +49,12 @@ async def async_step_user(self, user_input=None): """Step when user initializes a integration.""" self._errors = {} if user_input is not None: - if self._prt_in_configuration_exists(user_input): - return self.async_abort(reason="host_port_exists") + host = user_input[CONF_HOST] + port = user_input.get(CONF_PORT, DEFAULT_PORT) + await self.async_set_unique_id(f"{host}:{port}") + self._abort_if_unique_id_configured() if await self._test_connection(user_input): - host = user_input[CONF_HOST] - port = user_input.get(CONF_PORT, DEFAULT_PORT) title = host + (f":{port}" if port != DEFAULT_PORT else "") return self.async_create_entry( title=title, @@ -109,6 +91,4 @@ async def async_step_import(self, user_input=None): Only host was required in the yaml file all other fields are optional """ - if self._prt_in_configuration_exists(user_input): - return self.async_abort(reason="host_port_exists") return await self.async_step_user(user_input) diff --git a/homeassistant/components/cert_expiry/sensor.py b/homeassistant/components/cert_expiry/sensor.py index b0c393cf820a42..d4353904383961 100644 --- a/homeassistant/components/cert_expiry/sensor.py +++ b/homeassistant/components/cert_expiry/sensor.py @@ -68,6 +68,9 @@ async def async_setup_entry(hass, entry, async_add_entities): hostname = entry.data[CONF_HOST] port = entry.data[CONF_PORT] + if entry.unique_id is None: + hass.config_entries.async_update_entry(entry, unique_id=f"{hostname}:{port}") + try: days = await get_cert_time_to_expiry(hass, hostname, port) except TemporaryFailure as err: From 53485d9f120d6a48184fffc54d2edaf9557acea8 Mon Sep 17 00:00:00 2001 From: Jason Lawrence Date: Sun, 23 Feb 2020 23:41:38 -0600 Subject: [PATCH 06/15] Fix logic bugs found with tests --- homeassistant/components/cert_expiry/sensor.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/homeassistant/components/cert_expiry/sensor.py b/homeassistant/components/cert_expiry/sensor.py index d4353904383961..1529fbc7ff135f 100644 --- a/homeassistant/components/cert_expiry/sensor.py +++ b/homeassistant/components/cert_expiry/sensor.py @@ -78,7 +78,6 @@ async def async_setup_entry(hass, entry, async_add_entities): raise PlatformNotReady except ValidationFailure as err: error = err - pass async_add_entities( [SSLCertificate(hass, hostname, port, days, error)], False, @@ -96,13 +95,11 @@ def __init__(self, hass, server_name, server_port, days, error): self.server_port = server_port display_port = f":{server_port}" if server_port != DEFAULT_PORT else "" self._name = f"Cert Expiry ({self.server_name}{display_port})" - self._available = False + self._available = True self._error = error - self._state = None + self._state = days self._valid = False - if days is not None: - self._state = days - self._available = True + if error is None: self._valid = True @property From bfa2ffb296f47748cfe3d940c486e9fc9aa89074 Mon Sep 17 00:00:00 2001 From: Jason Lawrence Date: Sun, 23 Feb 2020 23:44:20 -0600 Subject: [PATCH 07/15] Rewrite tests to use config flow core interfaces, validate created sensors --- .coveragerc | 2 - .../cert_expiry/test_config_flow.py | 200 ++++++++++++------ 2 files changed, 139 insertions(+), 63 deletions(-) diff --git a/.coveragerc b/.coveragerc index 75153dd4e08cd8..7ec05c881a7c6a 100644 --- a/.coveragerc +++ b/.coveragerc @@ -107,8 +107,6 @@ omit = homeassistant/components/canary/alarm_control_panel.py homeassistant/components/canary/camera.py homeassistant/components/cast/* - homeassistant/components/cert_expiry/__init__.py - homeassistant/components/cert_expiry/errors.py homeassistant/components/cert_expiry/helper.py homeassistant/components/cert_expiry/sensor.py homeassistant/components/channels/* diff --git a/tests/components/cert_expiry/test_config_flow.py b/tests/components/cert_expiry/test_config_flow.py index fcac1c0747730e..c3d4e559a068df 100644 --- a/tests/components/cert_expiry/test_config_flow.py +++ b/tests/components/cert_expiry/test_config_flow.py @@ -6,10 +6,8 @@ from asynctest import patch as asyncpatch from homeassistant import data_entry_flow -from homeassistant.components.cert_expiry import config_flow -from homeassistant.components.cert_expiry.const import DEFAULT_PORT -from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_USER -from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT +from homeassistant.components.cert_expiry.const import DEFAULT_PORT, DOMAIN +from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT, STATE_UNAVAILABLE from tests.common import MockConfigEntry @@ -19,95 +17,173 @@ GET_CERT_TIME_METHOD = ( "homeassistant.components.cert_expiry.config_flow.get_cert_time_to_expiry" ) - - -def init_config_flow(hass, source=SOURCE_USER): - """Init a configuration flow.""" - flow = config_flow.CertexpiryConfigFlow() - flow.hass = hass - flow.context = {"source": source} - return flow +GET_SENSOR_CERT_TIME_METHOD = ( + "homeassistant.components.cert_expiry.sensor.get_cert_time_to_expiry" +) async def test_user(hass): """Test user config.""" - flow = init_config_flow(hass) - - result = await flow.async_step_user() + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["step_id"] == "user" - # test with all provided - with asyncpatch(GET_CERT_TIME_METHOD, return_value=1): - result = await flow.async_step_user({CONF_HOST: HOST, CONF_PORT: PORT}) + with asyncpatch(GET_CERT_TIME_METHOD): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input={CONF_HOST: HOST, CONF_PORT: PORT} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["title"] == HOST assert result["data"][CONF_HOST] == HOST assert result["data"][CONF_PORT] == PORT + assert result["result"].unique_id == f"{HOST}:{PORT}" + with asyncpatch(GET_SENSOR_CERT_TIME_METHOD, return_value=100): + await hass.async_block_till_done() -async def test_user_with_bad_cert(hass): - """Test user config.""" - flow = init_config_flow(hass) + state = hass.states.get("sensor.cert_expiry_example_com") + assert state is not None + assert state.state != STATE_UNAVAILABLE + assert state.state == "100" + assert state.attributes.get("error") == "None" + assert state.attributes.get("is_valid") - result = await flow.async_step_user() + +async def test_user_with_bad_cert(hass): + """Test user config with bad certificate.""" + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["step_id"] == "user" - # import with certification validation error with patch(GET_CERT_METHOD, side_effect=ssl.SSLError("some error")): - result = await flow.async_step_user({CONF_HOST: HOST, CONF_PORT: PORT}) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input={CONF_HOST: HOST, CONF_PORT: PORT} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["title"] == HOST assert result["data"][CONF_HOST] == HOST assert result["data"][CONF_PORT] == PORT + assert result["result"].unique_id == f"{HOST}:{PORT}" + with patch(GET_CERT_METHOD, side_effect=ssl.SSLError("some error")): + await hass.async_block_till_done() + + state = hass.states.get("sensor.cert_expiry_example_com") + assert state is not None + assert state.state != STATE_UNAVAILABLE + assert state.state == "0" + assert state.attributes.get("error") == "some error" + assert not state.attributes.get("is_valid") -async def test_import(hass): - """Test import step.""" - flow = init_config_flow(hass, source=SOURCE_IMPORT) - # import with only host +async def test_import_host_only(hass): + """Test import with host only.""" with asyncpatch(GET_CERT_TIME_METHOD, return_value=1): - result = await flow.async_step_import({CONF_HOST: HOST}) + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "import"}, data={CONF_HOST: HOST} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["title"] == HOST assert result["data"][CONF_HOST] == HOST assert result["data"][CONF_PORT] == DEFAULT_PORT + assert result["result"].unique_id == f"{HOST}:{DEFAULT_PORT}" + + with asyncpatch(GET_SENSOR_CERT_TIME_METHOD, return_value=100): + await hass.async_block_till_done() + state = hass.states.get("sensor.cert_expiry_example_com") + assert state is not None + assert state.state != STATE_UNAVAILABLE + assert state.attributes.get("error") == "None" + assert state.attributes.get("is_valid") + assert state.state == "100" + - # import with host and port +async def test_import_host_and_port(hass): + """Test import with host and port.""" with asyncpatch(GET_CERT_TIME_METHOD, return_value=1): - result = await flow.async_step_import({CONF_HOST: HOST, CONF_PORT: PORT}) + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": "import"}, + data={CONF_HOST: HOST, CONF_PORT: PORT}, + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["title"] == HOST assert result["data"][CONF_HOST] == HOST assert result["data"][CONF_PORT] == PORT + assert result["result"].unique_id == f"{HOST}:{PORT}" + + with asyncpatch(GET_SENSOR_CERT_TIME_METHOD, return_value=100): + await hass.async_block_till_done() + state = hass.states.get("sensor.cert_expiry_example_com") + assert state is not None + assert state.state != STATE_UNAVAILABLE + assert state.attributes.get("error") == "None" + assert state.attributes.get("is_valid") + assert state.state == "100" + + +async def test_import_non_default_port(hass): + """Test import with host and non-default port.""" + with asyncpatch(GET_CERT_TIME_METHOD): + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "import"}, data={CONF_HOST: HOST, CONF_PORT: 888} + ) - # import with host and non-default port - with asyncpatch(GET_CERT_TIME_METHOD, return_value=1): - result = await flow.async_step_import({CONF_HOST: HOST, CONF_PORT: 888}) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["title"] == f"{HOST}:888" assert result["data"][CONF_HOST] == HOST assert result["data"][CONF_PORT] == 888 + assert result["result"].unique_id == f"{HOST}:888" + + with asyncpatch(GET_SENSOR_CERT_TIME_METHOD, return_value=100): + await hass.async_block_till_done() + state = hass.states.get("sensor.cert_expiry_example_com_888") + assert state is not None + assert state.state != STATE_UNAVAILABLE + assert state.attributes.get("error") == "None" + assert state.attributes.get("is_valid") + assert state.state == "100" - # import legacy config with name + +async def test_import_with_name(hass): + """Test import with name (deprecated).""" with asyncpatch(GET_CERT_TIME_METHOD, return_value=1): - result = await flow.async_step_import( - {CONF_NAME: "legacy", CONF_HOST: HOST, CONF_PORT: PORT} + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": "import"}, + data={CONF_NAME: "legacy", CONF_HOST: HOST, CONF_PORT: PORT}, ) + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["title"] == HOST assert result["data"][CONF_HOST] == HOST assert result["data"][CONF_PORT] == PORT + assert result["result"].unique_id == f"{HOST}:{PORT}" + + with asyncpatch(GET_SENSOR_CERT_TIME_METHOD, return_value=100): + await hass.async_block_till_done() + state = hass.states.get("sensor.cert_expiry_example_com") + assert state is not None + assert state.state != STATE_UNAVAILABLE + assert state.attributes.get("error") == "None" + assert state.attributes.get("is_valid") + assert state.state == "100" async def test_bad_import(hass): """Test import step.""" - flow = init_config_flow(hass, source=SOURCE_IMPORT) - with patch(GET_CERT_METHOD, side_effect=ConnectionRefusedError()): - result = await flow.async_step_import({CONF_HOST: HOST}) + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "import"}, data={CONF_HOST: HOST} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT assert result["reason"] == "import_failed" @@ -115,45 +191,47 @@ async def test_bad_import(hass): async def test_abort_if_already_setup(hass): """Test we abort if the cert is already setup.""" MockConfigEntry( - domain="cert_expiry", data={CONF_PORT: DEFAULT_PORT, CONF_HOST: HOST}, + domain="cert_expiry", + data={CONF_HOST: HOST, CONF_PORT: PORT}, + unique_id=f"{HOST}:{PORT}", ).add_to_hass(hass) - # Test with import source - flow = init_config_flow(hass, source=SOURCE_IMPORT) - - # Should fail, same HOST and PORT (default) - result = await flow.async_step_import({CONF_HOST: HOST, CONF_PORT: DEFAULT_PORT}) + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "import"}, data={CONF_HOST: HOST, CONF_PORT: PORT} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT - assert result["reason"] == "host_port_exists" - - # Test with user source - flow = init_config_flow(hass) + assert result["reason"] == "already_configured" - result = await flow.async_step_user() - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "user" - - # Should fail, same HOST and PORT (default) - result = await flow.async_step_user({CONF_HOST: HOST, CONF_PORT: DEFAULT_PORT}) + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"}, data={CONF_HOST: HOST, CONF_PORT: PORT} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT - assert result["reason"] == "host_port_exists" + assert result["reason"] == "already_configured" async def test_abort_on_socket_failed(hass): """Test we abort of we have errors during socket creation.""" - flow = init_config_flow(hass) + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) with patch(GET_CERT_METHOD, side_effect=socket.gaierror()): - result = await flow.async_step_user({CONF_HOST: HOST}) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input={CONF_HOST: HOST} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["errors"] == {CONF_HOST: "resolve_failed"} with patch(GET_CERT_METHOD, side_effect=socket.timeout()): - result = await flow.async_step_user({CONF_HOST: HOST}) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input={CONF_HOST: HOST} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["errors"] == {CONF_HOST: "connection_timeout"} with patch(GET_CERT_METHOD, side_effect=ConnectionRefusedError): - result = await flow.async_step_user({CONF_HOST: HOST}) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input={CONF_HOST: HOST} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["errors"] == {CONF_HOST: "connection_refused"} From 2fe8c28a4f407fb8c7e35eeb54decfd7b2c2188b Mon Sep 17 00:00:00 2001 From: Jason Lawrence Date: Sun, 23 Feb 2020 23:52:54 -0600 Subject: [PATCH 08/15] Update strings --- homeassistant/components/cert_expiry/strings.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/cert_expiry/strings.json b/homeassistant/components/cert_expiry/strings.json index 286239fd5a5d49..4d4982a19af777 100644 --- a/homeassistant/components/cert_expiry/strings.json +++ b/homeassistant/components/cert_expiry/strings.json @@ -17,7 +17,7 @@ "connection_refused": "Connection refused when connecting to host" }, "abort": { - "host_port_exists": "This host and port combination is already configured", + "already_configured": "This host and port combination is already configured", "import_failed": "Import from config failed" } } From fc61049f14d309891b26a4f5c832b071a4de572b Mon Sep 17 00:00:00 2001 From: Jason Lawrence Date: Mon, 24 Feb 2020 00:30:05 -0600 Subject: [PATCH 09/15] Minor consistency fix --- tests/components/cert_expiry/test_config_flow.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/components/cert_expiry/test_config_flow.py b/tests/components/cert_expiry/test_config_flow.py index c3d4e559a068df..4a59f2924b4ec9 100644 --- a/tests/components/cert_expiry/test_config_flow.py +++ b/tests/components/cert_expiry/test_config_flow.py @@ -83,7 +83,7 @@ async def test_user_with_bad_cert(hass): async def test_import_host_only(hass): """Test import with host only.""" - with asyncpatch(GET_CERT_TIME_METHOD, return_value=1): + with asyncpatch(GET_CERT_TIME_METHOD): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": "import"}, data={CONF_HOST: HOST} ) @@ -106,7 +106,7 @@ async def test_import_host_only(hass): async def test_import_host_and_port(hass): """Test import with host and port.""" - with asyncpatch(GET_CERT_TIME_METHOD, return_value=1): + with asyncpatch(GET_CERT_TIME_METHOD): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": "import"}, @@ -154,7 +154,7 @@ async def test_import_non_default_port(hass): async def test_import_with_name(hass): """Test import with name (deprecated).""" - with asyncpatch(GET_CERT_TIME_METHOD, return_value=1): + with asyncpatch(GET_CERT_TIME_METHOD): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": "import"}, From e652e8d646be29d027b2be3e94ce00bfe48b53e0 Mon Sep 17 00:00:00 2001 From: Jason Lawrence Date: Mon, 24 Feb 2020 16:29:15 -0600 Subject: [PATCH 10/15] Review fixes, complete test coverage --- .coveragerc | 1 - .../components/cert_expiry/config_flow.py | 9 +- .../components/cert_expiry/sensor.py | 15 +- tests/components/cert_expiry/const.py | 3 + .../cert_expiry/test_config_flow.py | 115 ++++------ tests/components/cert_expiry/test_init.py | 87 ++++++++ tests/components/cert_expiry/test_sensors.py | 211 ++++++++++++++++++ 7 files changed, 356 insertions(+), 85 deletions(-) create mode 100644 tests/components/cert_expiry/const.py create mode 100644 tests/components/cert_expiry/test_init.py create mode 100644 tests/components/cert_expiry/test_sensors.py diff --git a/.coveragerc b/.coveragerc index 7ec05c881a7c6a..97e02025473eb3 100644 --- a/.coveragerc +++ b/.coveragerc @@ -108,7 +108,6 @@ omit = homeassistant/components/canary/camera.py homeassistant/components/cast/* homeassistant/components/cert_expiry/helper.py - homeassistant/components/cert_expiry/sensor.py homeassistant/components/channels/* homeassistant/components/cisco_ios/device_tracker.py homeassistant/components/cisco_mobility_express/device_tracker.py diff --git a/homeassistant/components/cert_expiry/config_flow.py b/homeassistant/components/cert_expiry/config_flow.py index f40f9970cd88b0..427033a2f148db 100644 --- a/homeassistant/components/cert_expiry/config_flow.py +++ b/homeassistant/components/cert_expiry/config_flow.py @@ -55,13 +55,10 @@ async def async_step_user(self, user_input=None): self._abort_if_unique_id_configured() if await self._test_connection(user_input): - title = host + (f":{port}" if port != DEFAULT_PORT else "") + title_port = f":{port}" if port != DEFAULT_PORT else "" + title = f"{host}{title_port}" return self.async_create_entry( - title=title, - data={ - CONF_HOST: user_input[CONF_HOST], - CONF_PORT: user_input.get(CONF_PORT, DEFAULT_PORT), - }, + title=title, data={CONF_HOST: host, CONF_PORT: port}, ) if ( # pylint: disable=no-member self.context["source"] == config_entries.SOURCE_IMPORT diff --git a/homeassistant/components/cert_expiry/sensor.py b/homeassistant/components/cert_expiry/sensor.py index 1529fbc7ff135f..a4b7789043357f 100644 --- a/homeassistant/components/cert_expiry/sensor.py +++ b/homeassistant/components/cert_expiry/sensor.py @@ -17,8 +17,7 @@ from homeassistant.exceptions import PlatformNotReady import homeassistant.helpers.config_validation as cv from homeassistant.helpers.entity import Entity -from homeassistant.helpers.event import async_track_point_in_utc_time -from homeassistant.util import dt as dt_util +from homeassistant.helpers.event import async_call_later from .const import DEFAULT_PORT, DOMAIN from .errors import TemporaryFailure, ValidationFailure @@ -46,8 +45,7 @@ async def async_setup_platform(hass, config, async_add_entities, discovery_info= @callback def schedule_import(_): """Schedule delayed import after HA is fully started.""" - import_time = dt_util.utcnow() + timedelta(seconds=10) - async_track_point_in_utc_time(hass, do_import, import_time) + async_call_later(hass, 10, do_import) @callback def do_import(_): @@ -80,7 +78,7 @@ async def async_setup_entry(hass, entry, async_add_entities): error = err async_add_entities( - [SSLCertificate(hass, hostname, port, days, error)], False, + [SSLCertificate(hostname, port, days, error)], False, ) return True @@ -88,9 +86,8 @@ async def async_setup_entry(hass, entry, async_add_entities): class SSLCertificate(Entity): """Implementation of the certificate expiry sensor.""" - def __init__(self, hass, server_name, server_port, days, error): + def __init__(self, server_name, server_port, days, error): """Initialize the sensor.""" - self.hass = hass self.server_name = server_name self.server_port = server_port display_port = f":{server_port}" if server_port != DEFAULT_PORT else "" @@ -141,8 +138,6 @@ async def async_update(self): except TemporaryFailure as err: _LOGGER.error(err) self._available = False - self._error = err - self._valid = False return except ValidationFailure as err: _LOGGER.error( @@ -158,8 +153,6 @@ async def async_update(self): "Unknown error checking %s:%s", self.server_name, self.server_port ) self._available = False - self._error = "Unknown" - self._valid = False return self._available = True diff --git a/tests/components/cert_expiry/const.py b/tests/components/cert_expiry/const.py new file mode 100644 index 00000000000000..9ddbeca61c336f --- /dev/null +++ b/tests/components/cert_expiry/const.py @@ -0,0 +1,3 @@ +"""Constants for cert_expiry tests.""" +PORT = 443 +HOST = "example.com" diff --git a/tests/components/cert_expiry/test_config_flow.py b/tests/components/cert_expiry/test_config_flow.py index 4a59f2924b4ec9..1b2cc175dcb71c 100644 --- a/tests/components/cert_expiry/test_config_flow.py +++ b/tests/components/cert_expiry/test_config_flow.py @@ -1,25 +1,16 @@ """Tests for the Cert Expiry config flow.""" import socket import ssl -from unittest.mock import patch -from asynctest import patch as asyncpatch +from asynctest import patch from homeassistant import data_entry_flow from homeassistant.components.cert_expiry.const import DEFAULT_PORT, DOMAIN -from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT, STATE_UNAVAILABLE +from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT -from tests.common import MockConfigEntry +from .const import HOST, PORT -PORT = 443 -HOST = "example.com" -GET_CERT_METHOD = "homeassistant.components.cert_expiry.helper.get_cert" -GET_CERT_TIME_METHOD = ( - "homeassistant.components.cert_expiry.config_flow.get_cert_time_to_expiry" -) -GET_SENSOR_CERT_TIME_METHOD = ( - "homeassistant.components.cert_expiry.sensor.get_cert_time_to_expiry" -) +from tests.common import MockConfigEntry async def test_user(hass): @@ -30,7 +21,9 @@ async def test_user(hass): assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["step_id"] == "user" - with asyncpatch(GET_CERT_TIME_METHOD): + with patch( + "homeassistant.components.cert_expiry.config_flow.get_cert_time_to_expiry" + ): result = await hass.config_entries.flow.async_configure( result["flow_id"], user_input={CONF_HOST: HOST, CONF_PORT: PORT} ) @@ -40,16 +33,9 @@ async def test_user(hass): assert result["data"][CONF_PORT] == PORT assert result["result"].unique_id == f"{HOST}:{PORT}" - with asyncpatch(GET_SENSOR_CERT_TIME_METHOD, return_value=100): + with patch("homeassistant.components.cert_expiry.sensor.async_setup_entry"): await hass.async_block_till_done() - state = hass.states.get("sensor.cert_expiry_example_com") - assert state is not None - assert state.state != STATE_UNAVAILABLE - assert state.state == "100" - assert state.attributes.get("error") == "None" - assert state.attributes.get("is_valid") - async def test_user_with_bad_cert(hass): """Test user config with bad certificate.""" @@ -59,7 +45,10 @@ async def test_user_with_bad_cert(hass): assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["step_id"] == "user" - with patch(GET_CERT_METHOD, side_effect=ssl.SSLError("some error")): + with patch( + "homeassistant.components.cert_expiry.helper.get_cert", + side_effect=ssl.SSLError("some error"), + ): result = await hass.config_entries.flow.async_configure( result["flow_id"], user_input={CONF_HOST: HOST, CONF_PORT: PORT} ) @@ -70,20 +59,16 @@ async def test_user_with_bad_cert(hass): assert result["data"][CONF_PORT] == PORT assert result["result"].unique_id == f"{HOST}:{PORT}" - with patch(GET_CERT_METHOD, side_effect=ssl.SSLError("some error")): + with patch("homeassistant.components.cert_expiry.sensor.async_setup_entry"): await hass.async_block_till_done() - state = hass.states.get("sensor.cert_expiry_example_com") - assert state is not None - assert state.state != STATE_UNAVAILABLE - assert state.state == "0" - assert state.attributes.get("error") == "some error" - assert not state.attributes.get("is_valid") - async def test_import_host_only(hass): """Test import with host only.""" - with asyncpatch(GET_CERT_TIME_METHOD): + with patch( + "homeassistant.components.cert_expiry.config_flow.get_cert_time_to_expiry", + return_value=1, + ): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": "import"}, data={CONF_HOST: HOST} ) @@ -94,19 +79,16 @@ async def test_import_host_only(hass): assert result["data"][CONF_PORT] == DEFAULT_PORT assert result["result"].unique_id == f"{HOST}:{DEFAULT_PORT}" - with asyncpatch(GET_SENSOR_CERT_TIME_METHOD, return_value=100): + with patch("homeassistant.components.cert_expiry.sensor.async_setup_entry"): await hass.async_block_till_done() - state = hass.states.get("sensor.cert_expiry_example_com") - assert state is not None - assert state.state != STATE_UNAVAILABLE - assert state.attributes.get("error") == "None" - assert state.attributes.get("is_valid") - assert state.state == "100" async def test_import_host_and_port(hass): """Test import with host and port.""" - with asyncpatch(GET_CERT_TIME_METHOD): + with patch( + "homeassistant.components.cert_expiry.config_flow.get_cert_time_to_expiry", + return_value=1, + ): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": "import"}, @@ -119,19 +101,15 @@ async def test_import_host_and_port(hass): assert result["data"][CONF_PORT] == PORT assert result["result"].unique_id == f"{HOST}:{PORT}" - with asyncpatch(GET_SENSOR_CERT_TIME_METHOD, return_value=100): + with patch("homeassistant.components.cert_expiry.sensor.async_setup_entry"): await hass.async_block_till_done() - state = hass.states.get("sensor.cert_expiry_example_com") - assert state is not None - assert state.state != STATE_UNAVAILABLE - assert state.attributes.get("error") == "None" - assert state.attributes.get("is_valid") - assert state.state == "100" async def test_import_non_default_port(hass): """Test import with host and non-default port.""" - with asyncpatch(GET_CERT_TIME_METHOD): + with patch( + "homeassistant.components.cert_expiry.config_flow.get_cert_time_to_expiry" + ): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": "import"}, data={CONF_HOST: HOST, CONF_PORT: 888} ) @@ -142,19 +120,16 @@ async def test_import_non_default_port(hass): assert result["data"][CONF_PORT] == 888 assert result["result"].unique_id == f"{HOST}:888" - with asyncpatch(GET_SENSOR_CERT_TIME_METHOD, return_value=100): + with patch("homeassistant.components.cert_expiry.sensor.async_setup_entry"): await hass.async_block_till_done() - state = hass.states.get("sensor.cert_expiry_example_com_888") - assert state is not None - assert state.state != STATE_UNAVAILABLE - assert state.attributes.get("error") == "None" - assert state.attributes.get("is_valid") - assert state.state == "100" async def test_import_with_name(hass): """Test import with name (deprecated).""" - with asyncpatch(GET_CERT_TIME_METHOD): + with patch( + "homeassistant.components.cert_expiry.config_flow.get_cert_time_to_expiry", + return_value=1, + ): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": "import"}, @@ -167,19 +142,16 @@ async def test_import_with_name(hass): assert result["data"][CONF_PORT] == PORT assert result["result"].unique_id == f"{HOST}:{PORT}" - with asyncpatch(GET_SENSOR_CERT_TIME_METHOD, return_value=100): + with patch("homeassistant.components.cert_expiry.sensor.async_setup_entry"): await hass.async_block_till_done() - state = hass.states.get("sensor.cert_expiry_example_com") - assert state is not None - assert state.state != STATE_UNAVAILABLE - assert state.attributes.get("error") == "None" - assert state.attributes.get("is_valid") - assert state.state == "100" async def test_bad_import(hass): """Test import step.""" - with patch(GET_CERT_METHOD, side_effect=ConnectionRefusedError()): + with patch( + "homeassistant.components.cert_expiry.helper.get_cert", + side_effect=ConnectionRefusedError(), + ): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": "import"}, data={CONF_HOST: HOST} ) @@ -215,21 +187,30 @@ async def test_abort_on_socket_failed(hass): DOMAIN, context={"source": "user"} ) - with patch(GET_CERT_METHOD, side_effect=socket.gaierror()): + with patch( + "homeassistant.components.cert_expiry.helper.get_cert", + side_effect=socket.gaierror(), + ): result = await hass.config_entries.flow.async_configure( result["flow_id"], user_input={CONF_HOST: HOST} ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["errors"] == {CONF_HOST: "resolve_failed"} - with patch(GET_CERT_METHOD, side_effect=socket.timeout()): + with patch( + "homeassistant.components.cert_expiry.helper.get_cert", + side_effect=socket.timeout(), + ): result = await hass.config_entries.flow.async_configure( result["flow_id"], user_input={CONF_HOST: HOST} ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["errors"] == {CONF_HOST: "connection_timeout"} - with patch(GET_CERT_METHOD, side_effect=ConnectionRefusedError): + with patch( + "homeassistant.components.cert_expiry.helper.get_cert", + side_effect=ConnectionRefusedError, + ): result = await hass.config_entries.flow.async_configure( result["flow_id"], user_input={CONF_HOST: HOST} ) diff --git a/tests/components/cert_expiry/test_init.py b/tests/components/cert_expiry/test_init.py new file mode 100644 index 00000000000000..580712fba76e66 --- /dev/null +++ b/tests/components/cert_expiry/test_init.py @@ -0,0 +1,87 @@ +"""Tests for the Cert Expiry sensors.""" +from datetime import timedelta + +from asynctest import patch + +from homeassistant.components import cert_expiry +from homeassistant.components.cert_expiry.const import DOMAIN +from homeassistant.components.sensor import DOMAIN as SENSOR_DOMAIN +from homeassistant.const import CONF_HOST, CONF_PORT, EVENT_HOMEASSISTANT_START +from homeassistant.setup import async_setup_component +import homeassistant.util.dt as dt_util + +from .const import HOST, PORT + +from tests.common import MockConfigEntry, async_fire_time_changed + + +async def test_setup_with_config(hass): + """Test setup component with config.""" + config = { + SENSOR_DOMAIN: [ + {"platform": DOMAIN, CONF_HOST: HOST, CONF_PORT: PORT}, + {"platform": DOMAIN, CONF_HOST: HOST, CONF_PORT: 888}, + ], + } + assert await async_setup_component(hass, SENSOR_DOMAIN, config) is True + await hass.async_block_till_done() + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + next_update = dt_util.utcnow() + timedelta(seconds=20) + async_fire_time_changed(hass, next_update) + + with patch( + "homeassistant.components.cert_expiry.config_flow.get_cert_time_to_expiry", + return_value=100, + ), patch( + "homeassistant.components.cert_expiry.sensor.get_cert_time_to_expiry", + return_value=100, + ): + await hass.async_block_till_done() + + assert len(hass.config_entries.async_entries(DOMAIN)) == 2 + + +async def test_update_unique_id(hass): + """Test updating a config entry without a unique_id.""" + entry = MockConfigEntry( + domain="cert_expiry", data={CONF_HOST: HOST, CONF_PORT: PORT}, + ) + assert not entry.unique_id + + with patch( + "homeassistant.components.cert_expiry.sensor.get_cert_time_to_expiry", + return_value=100, + ): + entry.add_to_hass(hass) + assert await cert_expiry.async_setup_entry(hass, entry) is True + await hass.async_block_till_done() + + assert entry.unique_id == f"{HOST}:{PORT}" + + +async def test_unload_config_entry(hass): + """Test unloading a config entry.""" + entry = MockConfigEntry( + domain="cert_expiry", + data={CONF_HOST: HOST, CONF_PORT: PORT}, + unique_id=f"{HOST}:{PORT}", + ) + + with patch( + "homeassistant.components.cert_expiry.sensor.get_cert_time_to_expiry", + return_value=100, + ): + entry.add_to_hass(hass) + assert await cert_expiry.async_setup_entry(hass, entry) is True + await hass.async_block_till_done() + + state = hass.states.get("sensor.cert_expiry_example_com") + assert state.state == "100" + assert state.attributes.get("error") == "None" + assert state.attributes.get("is_valid") + + assert await cert_expiry.async_unload_entry(hass, entry) + + state = hass.states.get("sensor.cert_expiry_example_com") + assert state is None diff --git a/tests/components/cert_expiry/test_sensors.py b/tests/components/cert_expiry/test_sensors.py new file mode 100644 index 00000000000000..6594b0988e723a --- /dev/null +++ b/tests/components/cert_expiry/test_sensors.py @@ -0,0 +1,211 @@ +"""Tests for the Cert Expiry sensors.""" +from datetime import timedelta +import socket +import ssl + +from asynctest import patch + +from homeassistant.const import CONF_HOST, CONF_PORT, STATE_UNAVAILABLE +import homeassistant.util.dt as dt_util + +from .const import HOST, PORT + +from tests.common import MockConfigEntry, async_fire_time_changed + + +async def test_async_setup_entry(hass): + """Test async_setup_entry.""" + entry = MockConfigEntry( + domain="cert_expiry", + data={CONF_HOST: HOST, CONF_PORT: PORT}, + unique_id=f"{HOST}:{PORT}", + ) + + with patch( + "homeassistant.components.cert_expiry.sensor.get_cert_time_to_expiry", + return_value=100, + ): + entry.add_to_hass(hass) + assert await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + state = hass.states.get("sensor.cert_expiry_example_com") + assert state is not None + assert state.state != STATE_UNAVAILABLE + assert state.state == "100" + assert state.attributes.get("error") == "None" + assert state.attributes.get("is_valid") + + +async def test_async_setup_entry_bad_cert(hass): + """Test async_setup_entry with a bad/expired cert.""" + entry = MockConfigEntry( + domain="cert_expiry", + data={CONF_HOST: HOST, CONF_PORT: PORT}, + unique_id=f"{HOST}:{PORT}", + ) + + with patch( + "homeassistant.components.cert_expiry.helper.get_cert", + side_effect=ssl.SSLError("some error"), + ): + entry.add_to_hass(hass) + assert await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + state = hass.states.get("sensor.cert_expiry_example_com") + assert state is not None + assert state.state != STATE_UNAVAILABLE + assert state.state == "0" + assert state.attributes.get("error") == "some error" + assert not state.attributes.get("is_valid") + + +async def test_async_setup_entry_host_unavailable(hass): + """Test async_setup_entry when host is unavailable.""" + entry = MockConfigEntry( + domain="cert_expiry", + data={CONF_HOST: HOST, CONF_PORT: PORT}, + unique_id=f"{HOST}:{PORT}", + ) + + with patch( + "homeassistant.components.cert_expiry.helper.get_cert", + side_effect=socket.gaierror, + ): + entry.add_to_hass(hass) + assert await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + state = hass.states.get("sensor.cert_expiry_example_com") + assert state is None + + next_update = dt_util.utcnow() + timedelta(seconds=45) + async_fire_time_changed(hass, next_update) + with patch( + "homeassistant.components.cert_expiry.helper.get_cert", + side_effect=socket.gaierror, + ): + await hass.async_block_till_done() + + state = hass.states.get("sensor.cert_expiry_example_com") + assert state is None + + +async def test_update_sensor(hass): + """Test async_update for sensor.""" + entry = MockConfigEntry( + domain="cert_expiry", + data={CONF_HOST: HOST, CONF_PORT: PORT}, + unique_id=f"{HOST}:{PORT}", + ) + + with patch( + "homeassistant.components.cert_expiry.sensor.get_cert_time_to_expiry", + return_value=100, + ): + entry.add_to_hass(hass) + assert await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + state = hass.states.get("sensor.cert_expiry_example_com") + assert state is not None + assert state.state != STATE_UNAVAILABLE + assert state.state == "100" + assert state.attributes.get("error") == "None" + assert state.attributes.get("is_valid") + + next_update = dt_util.utcnow() + timedelta(hours=12) + async_fire_time_changed(hass, next_update) + + with patch( + "homeassistant.components.cert_expiry.sensor.get_cert_time_to_expiry", + return_value=99, + ): + await hass.async_block_till_done() + + state = hass.states.get("sensor.cert_expiry_example_com") + assert state is not None + assert state.state != STATE_UNAVAILABLE + assert state.state == "99" + assert state.attributes.get("error") == "None" + assert state.attributes.get("is_valid") + + +async def test_update_sensor_network_errors(hass): + """Test async_update for sensor.""" + entry = MockConfigEntry( + domain="cert_expiry", + data={CONF_HOST: HOST, CONF_PORT: PORT}, + unique_id=f"{HOST}:{PORT}", + ) + + with patch( + "homeassistant.components.cert_expiry.sensor.get_cert_time_to_expiry", + return_value=100, + ): + entry.add_to_hass(hass) + assert await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + state = hass.states.get("sensor.cert_expiry_example_com") + assert state is not None + assert state.state != STATE_UNAVAILABLE + assert state.state == "100" + assert state.attributes.get("error") == "None" + assert state.attributes.get("is_valid") + + next_update = dt_util.utcnow() + timedelta(hours=12) + async_fire_time_changed(hass, next_update) + + with patch( + "homeassistant.components.cert_expiry.helper.get_cert", + side_effect=socket.gaierror, + ): + await hass.async_block_till_done() + + state = hass.states.get("sensor.cert_expiry_example_com") + assert state.state == STATE_UNAVAILABLE + + next_update = dt_util.utcnow() + timedelta(hours=12) + async_fire_time_changed(hass, next_update) + + with patch( + "homeassistant.components.cert_expiry.sensor.get_cert_time_to_expiry", + return_value=99, + ): + await hass.async_block_till_done() + + state = hass.states.get("sensor.cert_expiry_example_com") + assert state is not None + assert state.state != STATE_UNAVAILABLE + assert state.state == "99" + assert state.attributes.get("error") == "None" + assert state.attributes.get("is_valid") + + next_update = dt_util.utcnow() + timedelta(hours=12) + async_fire_time_changed(hass, next_update) + + with patch( + "homeassistant.components.cert_expiry.helper.get_cert", + side_effect=ssl.SSLError("something bad"), + ): + await hass.async_block_till_done() + + state = hass.states.get("sensor.cert_expiry_example_com") + assert state is not None + assert state.state != STATE_UNAVAILABLE + assert state.state == "0" + assert state.attributes.get("error") == "something bad" + assert not state.attributes.get("is_valid") + + next_update = dt_util.utcnow() + timedelta(hours=12) + async_fire_time_changed(hass, next_update) + + with patch( + "homeassistant.components.cert_expiry.helper.get_cert", side_effect=Exception() + ): + await hass.async_block_till_done() + + state = hass.states.get("sensor.cert_expiry_example_com") + assert state.state == STATE_UNAVAILABLE From f38181d363a2d3b158c2a87e4ca2bcc2e7e3e0e3 Mon Sep 17 00:00:00 2001 From: Jason Lawrence Date: Mon, 24 Feb 2020 16:49:07 -0600 Subject: [PATCH 11/15] Move error handling to helper --- .../components/cert_expiry/config_flow.py | 10 ++-------- homeassistant/components/cert_expiry/helper.py | 16 ++++++++-------- homeassistant/components/cert_expiry/sensor.py | 4 ++-- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/homeassistant/components/cert_expiry/config_flow.py b/homeassistant/components/cert_expiry/config_flow.py index 427033a2f148db..b22555daeedb0f 100644 --- a/homeassistant/components/cert_expiry/config_flow.py +++ b/homeassistant/components/cert_expiry/config_flow.py @@ -1,6 +1,5 @@ """Config flow for the Cert Expiry platform.""" import logging -import socket import voluptuous as vol @@ -34,13 +33,8 @@ async def _test_connection(self, user_input=None): ) return True except TemporaryFailure as err: - cause = type(err.__cause__) - if cause is socket.gaierror: - self._errors[CONF_HOST] = "resolve_failed" - elif cause is socket.timeout: - self._errors[CONF_HOST] = "connection_timeout" - elif cause is ConnectionRefusedError: - self._errors[CONF_HOST] = "connection_refused" + cause = err.args[1] + self._errors[CONF_HOST] = cause except ValidationFailure: return True return False diff --git a/homeassistant/components/cert_expiry/helper.py b/homeassistant/components/cert_expiry/helper.py index 9044b82a83e2b1..bcdb515febc535 100644 --- a/homeassistant/components/cert_expiry/helper.py +++ b/homeassistant/components/cert_expiry/helper.py @@ -22,16 +22,16 @@ async def get_cert_time_to_expiry(hass, hostname, port): """Return the certificate's time to expiry in days.""" try: cert = await hass.async_add_executor_job(get_cert, hostname, port) - except socket.gaierror as err: - raise TemporaryFailure(f"Cannot resolve hostname: {hostname}") from err - except socket.timeout as err: + except socket.gaierror: + raise TemporaryFailure(f"Cannot resolve hostname: {hostname}", "resolve_failed") + except socket.timeout: raise TemporaryFailure( - f"Connection timeout with server: {hostname}:{port}" - ) from err - except ConnectionRefusedError as err: + f"Connection timeout with server: {hostname}:{port}", "connection_timeout" + ) + except ConnectionRefusedError: raise TemporaryFailure( - f"Connection refused by server: {hostname}:{port}" - ) from err + f"Connection refused by server: {hostname}:{port}", "connection_refused" + ) except ssl.CertificateError as err: raise ValidationFailure(err.verify_message) except ssl.SSLError as err: diff --git a/homeassistant/components/cert_expiry/sensor.py b/homeassistant/components/cert_expiry/sensor.py index a4b7789043357f..b429282773fd15 100644 --- a/homeassistant/components/cert_expiry/sensor.py +++ b/homeassistant/components/cert_expiry/sensor.py @@ -72,7 +72,7 @@ async def async_setup_entry(hass, entry, async_add_entities): try: days = await get_cert_time_to_expiry(hass, hostname, port) except TemporaryFailure as err: - _LOGGER.error(err) + _LOGGER.error(err.args[0]) raise PlatformNotReady except ValidationFailure as err: error = err @@ -136,7 +136,7 @@ async def async_update(self): self.hass, self.server_name, self.server_port ) except TemporaryFailure as err: - _LOGGER.error(err) + _LOGGER.error(err.args[0]) self._available = False return except ValidationFailure as err: From 60b4b931bc90fdc4f44bfcbb5155de5090b806c6 Mon Sep 17 00:00:00 2001 From: Jason Lawrence Date: Mon, 24 Feb 2020 22:20:31 -0600 Subject: [PATCH 12/15] Subclass exceptions --- .../components/cert_expiry/config_flow.py | 16 ++++++++++++---- homeassistant/components/cert_expiry/errors.py | 12 ++++++++++++ homeassistant/components/cert_expiry/helper.py | 17 +++++++++-------- homeassistant/components/cert_expiry/sensor.py | 2 +- 4 files changed, 34 insertions(+), 13 deletions(-) diff --git a/homeassistant/components/cert_expiry/config_flow.py b/homeassistant/components/cert_expiry/config_flow.py index b22555daeedb0f..3f77701906f9b6 100644 --- a/homeassistant/components/cert_expiry/config_flow.py +++ b/homeassistant/components/cert_expiry/config_flow.py @@ -7,7 +7,12 @@ from homeassistant.const import CONF_HOST, CONF_PORT from .const import DEFAULT_PORT, DOMAIN # pylint: disable=unused-import -from .errors import TemporaryFailure, ValidationFailure +from .errors import ( + ConnectionRefused, + ConnectionTimeout, + ResolveFailed, + ValidationFailure, +) from .helper import get_cert_time_to_expiry _LOGGER = logging.getLogger(__name__) @@ -32,9 +37,12 @@ async def _test_connection(self, user_input=None): user_input.get(CONF_PORT, DEFAULT_PORT), ) return True - except TemporaryFailure as err: - cause = err.args[1] - self._errors[CONF_HOST] = cause + except ResolveFailed: + self._errors[CONF_HOST] = "resolve_failed" + except ConnectionTimeout: + self._errors[CONF_HOST] = "connection_timeout" + except ConnectionRefused: + self._errors[CONF_HOST] = "connection_refused" except ValidationFailure: return True return False diff --git a/homeassistant/components/cert_expiry/errors.py b/homeassistant/components/cert_expiry/errors.py index dadb09f395d450..a3b73c84f2a82b 100644 --- a/homeassistant/components/cert_expiry/errors.py +++ b/homeassistant/components/cert_expiry/errors.py @@ -12,3 +12,15 @@ class TemporaryFailure(CertExpiryException): class ValidationFailure(CertExpiryException): """Certificate validation failure has occurred.""" + + +class ResolveFailed(TemporaryFailure): + """Name resolution failed.""" + + +class ConnectionTimeout(TemporaryFailure): + """Network connection timed out.""" + + +class ConnectionRefused(TemporaryFailure): + """Network connection refused.""" diff --git a/homeassistant/components/cert_expiry/helper.py b/homeassistant/components/cert_expiry/helper.py index bcdb515febc535..bb9f2762f3a900 100644 --- a/homeassistant/components/cert_expiry/helper.py +++ b/homeassistant/components/cert_expiry/helper.py @@ -4,7 +4,12 @@ import ssl from .const import TIMEOUT -from .errors import TemporaryFailure, ValidationFailure +from .errors import ( + ConnectionRefused, + ConnectionTimeout, + ResolveFailed, + ValidationFailure, +) def get_cert(host, port): @@ -23,15 +28,11 @@ async def get_cert_time_to_expiry(hass, hostname, port): try: cert = await hass.async_add_executor_job(get_cert, hostname, port) except socket.gaierror: - raise TemporaryFailure(f"Cannot resolve hostname: {hostname}", "resolve_failed") + raise ResolveFailed(f"Cannot resolve hostname: {hostname}") except socket.timeout: - raise TemporaryFailure( - f"Connection timeout with server: {hostname}:{port}", "connection_timeout" - ) + raise ConnectionTimeout(f"Connection timeout with server: {hostname}:{port}") except ConnectionRefusedError: - raise TemporaryFailure( - f"Connection refused by server: {hostname}:{port}", "connection_refused" - ) + raise ConnectionRefused(f"Connection refused by server: {hostname}:{port}") except ssl.CertificateError as err: raise ValidationFailure(err.verify_message) except ssl.SSLError as err: diff --git a/homeassistant/components/cert_expiry/sensor.py b/homeassistant/components/cert_expiry/sensor.py index b429282773fd15..39ec2c35ac7612 100644 --- a/homeassistant/components/cert_expiry/sensor.py +++ b/homeassistant/components/cert_expiry/sensor.py @@ -72,7 +72,7 @@ async def async_setup_entry(hass, entry, async_add_entities): try: days = await get_cert_time_to_expiry(hass, hostname, port) except TemporaryFailure as err: - _LOGGER.error(err.args[0]) + _LOGGER.error(err) raise PlatformNotReady except ValidationFailure as err: error = err From 91a7eb9280574fe44e1f97721a003cec94f6b22b Mon Sep 17 00:00:00 2001 From: Jason Lawrence Date: Mon, 24 Feb 2020 22:22:29 -0600 Subject: [PATCH 13/15] Better tests --- tests/components/cert_expiry/test_init.py | 31 ++++++++++++++--------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/tests/components/cert_expiry/test_init.py b/tests/components/cert_expiry/test_init.py index 580712fba76e66..2c92282f65089e 100644 --- a/tests/components/cert_expiry/test_init.py +++ b/tests/components/cert_expiry/test_init.py @@ -3,9 +3,9 @@ from asynctest import patch -from homeassistant.components import cert_expiry from homeassistant.components.cert_expiry.const import DOMAIN from homeassistant.components.sensor import DOMAIN as SENSOR_DOMAIN +from homeassistant.config_entries import ENTRY_STATE_LOADED, ENTRY_STATE_NOT_LOADED from homeassistant.const import CONF_HOST, CONF_PORT, EVENT_HOMEASSISTANT_START from homeassistant.setup import async_setup_component import homeassistant.util.dt as dt_util @@ -44,44 +44,51 @@ async def test_setup_with_config(hass): async def test_update_unique_id(hass): """Test updating a config entry without a unique_id.""" - entry = MockConfigEntry( - domain="cert_expiry", data={CONF_HOST: HOST, CONF_PORT: PORT}, - ) - assert not entry.unique_id + entry = MockConfigEntry(domain=DOMAIN, data={CONF_HOST: HOST, CONF_PORT: PORT}) + entry.add_to_hass(hass) + + config_entries = hass.config_entries.async_entries(DOMAIN) + assert len(config_entries) == 1 + assert not config_entries[0].unique_id with patch( "homeassistant.components.cert_expiry.sensor.get_cert_time_to_expiry", return_value=100, ): - entry.add_to_hass(hass) - assert await cert_expiry.async_setup_entry(hass, entry) is True + assert await async_setup_component(hass, DOMAIN, {}) is True await hass.async_block_till_done() - assert entry.unique_id == f"{HOST}:{PORT}" + assert config_entries[0].state == ENTRY_STATE_LOADED + assert config_entries[0].unique_id == f"{HOST}:{PORT}" async def test_unload_config_entry(hass): """Test unloading a config entry.""" entry = MockConfigEntry( - domain="cert_expiry", + domain=DOMAIN, data={CONF_HOST: HOST, CONF_PORT: PORT}, unique_id=f"{HOST}:{PORT}", ) + entry.add_to_hass(hass) + + config_entries = hass.config_entries.async_entries(DOMAIN) + assert len(config_entries) == 1 with patch( "homeassistant.components.cert_expiry.sensor.get_cert_time_to_expiry", return_value=100, ): - entry.add_to_hass(hass) - assert await cert_expiry.async_setup_entry(hass, entry) is True + assert await async_setup_component(hass, DOMAIN, {}) is True await hass.async_block_till_done() + assert config_entries[0].state == ENTRY_STATE_LOADED state = hass.states.get("sensor.cert_expiry_example_com") assert state.state == "100" assert state.attributes.get("error") == "None" assert state.attributes.get("is_valid") - assert await cert_expiry.async_unload_entry(hass, entry) + await hass.config_entries.async_unload(config_entries[0].entry_id) + assert config_entries[0].state == ENTRY_STATE_NOT_LOADED state = hass.states.get("sensor.cert_expiry_example_com") assert state is None From 42d872e436b66d464c5e47a71e4ced9115725312 Mon Sep 17 00:00:00 2001 From: Jason Lawrence Date: Tue, 25 Feb 2020 08:27:09 -0600 Subject: [PATCH 14/15] Use first object reference --- tests/components/cert_expiry/test_init.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/components/cert_expiry/test_init.py b/tests/components/cert_expiry/test_init.py index 2c92282f65089e..13c7caa775be68 100644 --- a/tests/components/cert_expiry/test_init.py +++ b/tests/components/cert_expiry/test_init.py @@ -49,7 +49,8 @@ async def test_update_unique_id(hass): config_entries = hass.config_entries.async_entries(DOMAIN) assert len(config_entries) == 1 - assert not config_entries[0].unique_id + assert entry is config_entries[0] + assert not entry.unique_id with patch( "homeassistant.components.cert_expiry.sensor.get_cert_time_to_expiry", @@ -58,8 +59,8 @@ async def test_update_unique_id(hass): assert await async_setup_component(hass, DOMAIN, {}) is True await hass.async_block_till_done() - assert config_entries[0].state == ENTRY_STATE_LOADED - assert config_entries[0].unique_id == f"{HOST}:{PORT}" + assert entry.state == ENTRY_STATE_LOADED + assert entry.unique_id == f"{HOST}:{PORT}" async def test_unload_config_entry(hass): @@ -73,6 +74,7 @@ async def test_unload_config_entry(hass): config_entries = hass.config_entries.async_entries(DOMAIN) assert len(config_entries) == 1 + assert entry is config_entries[0] with patch( "homeassistant.components.cert_expiry.sensor.get_cert_time_to_expiry", @@ -81,14 +83,14 @@ async def test_unload_config_entry(hass): assert await async_setup_component(hass, DOMAIN, {}) is True await hass.async_block_till_done() - assert config_entries[0].state == ENTRY_STATE_LOADED + assert entry.state == ENTRY_STATE_LOADED state = hass.states.get("sensor.cert_expiry_example_com") assert state.state == "100" assert state.attributes.get("error") == "None" assert state.attributes.get("is_valid") - await hass.config_entries.async_unload(config_entries[0].entry_id) + await hass.config_entries.async_unload(entry.entry_id) - assert config_entries[0].state == ENTRY_STATE_NOT_LOADED + assert entry.state == ENTRY_STATE_NOT_LOADED state = hass.states.get("sensor.cert_expiry_example_com") assert state is None From 2f6306943d9e0f792cd35bee8167cbec2ad2ef1c Mon Sep 17 00:00:00 2001 From: Jason Lawrence Date: Sun, 1 Mar 2020 22:09:26 -0600 Subject: [PATCH 15/15] Fix docstring --- tests/components/cert_expiry/test_init.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/components/cert_expiry/test_init.py b/tests/components/cert_expiry/test_init.py index 13c7caa775be68..d4419b48370ac6 100644 --- a/tests/components/cert_expiry/test_init.py +++ b/tests/components/cert_expiry/test_init.py @@ -1,4 +1,4 @@ -"""Tests for the Cert Expiry sensors.""" +"""Tests for Cert Expiry setup.""" from datetime import timedelta from asynctest import patch