From 07da0cfb2b54a87d8ac52d1ea9bdc0f920e7cf79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ab=C3=ADlio=20Costa?= Date: Wed, 1 Oct 2025 11:11:00 +0100 Subject: [PATCH] Stop writing to config dir log file on supervised install (#146675) Co-authored-by: Martin Hjelmare --- homeassistant/bootstrap.py | 52 ++++++++++++++---------- tests/test_bootstrap.py | 81 +++++++++++++++++++++++++++++++------- 2 files changed, 98 insertions(+), 35 deletions(-) diff --git a/homeassistant/bootstrap.py b/homeassistant/bootstrap.py index 4e49d6cec7e..24268f4f4e2 100644 --- a/homeassistant/bootstrap.py +++ b/homeassistant/bootstrap.py @@ -616,34 +616,44 @@ async def async_enable_logging( ), ) - # Log errors to a file if we have write access to file or config dir + logger = logging.getLogger() + logger.setLevel(logging.INFO if verbose else logging.WARNING) + if log_file is None: - err_log_path = hass.config.path(ERROR_LOG_FILENAME) + default_log_path = hass.config.path(ERROR_LOG_FILENAME) + if "SUPERVISOR" in os.environ: + _LOGGER.info("Running in Supervisor, not logging to file") + # Rename the default log file if it exists, since previous versions created + # it even on Supervisor + if os.path.isfile(default_log_path): + with contextlib.suppress(OSError): + os.rename(default_log_path, f"{default_log_path}.old") + err_log_path = None + else: + err_log_path = default_log_path else: err_log_path = os.path.abspath(log_file) - err_path_exists = os.path.isfile(err_log_path) - err_dir = os.path.dirname(err_log_path) + if err_log_path: + err_path_exists = os.path.isfile(err_log_path) + err_dir = os.path.dirname(err_log_path) - # Check if we can write to the error log if it exists or that - # we can create files in the containing directory if not. - if (err_path_exists and os.access(err_log_path, os.W_OK)) or ( - not err_path_exists and os.access(err_dir, os.W_OK) - ): - err_handler = await hass.async_add_executor_job( - _create_log_file, err_log_path, log_rotate_days - ) + # Check if we can write to the error log if it exists or that + # we can create files in the containing directory if not. + if (err_path_exists and os.access(err_log_path, os.W_OK)) or ( + not err_path_exists and os.access(err_dir, os.W_OK) + ): + err_handler = await hass.async_add_executor_job( + _create_log_file, err_log_path, log_rotate_days + ) - err_handler.setFormatter(logging.Formatter(fmt, datefmt=FORMAT_DATETIME)) + err_handler.setFormatter(logging.Formatter(fmt, datefmt=FORMAT_DATETIME)) + logger.addHandler(err_handler) - logger = logging.getLogger() - logger.addHandler(err_handler) - logger.setLevel(logging.INFO if verbose else logging.WARNING) - - # Save the log file location for access by other components. - hass.data[DATA_LOGGING] = err_log_path - else: - _LOGGER.error("Unable to set up error log %s (access denied)", err_log_path) + # Save the log file location for access by other components. + hass.data[DATA_LOGGING] = err_log_path + else: + _LOGGER.error("Unable to set up error log %s (access denied)", err_log_path) async_activate_log_queue_handler(hass) diff --git a/tests/test_bootstrap.py b/tests/test_bootstrap.py index 9e1f246b551..604b375d299 100644 --- a/tests/test_bootstrap.py +++ b/tests/test_bootstrap.py @@ -38,6 +38,17 @@ from .common import ( VERSION_PATH = os.path.join(get_test_config_dir(), config_util.VERSION_FILE) +CONFIG_LOG_FILE = get_test_config_dir("home-assistant.log") +ARG_LOG_FILE = "test.log" + + +def cleanup_log_files() -> None: + """Remove all log files.""" + for f in glob.glob(f"{CONFIG_LOG_FILE}*"): + os.remove(f) + for f in glob.glob(f"{ARG_LOG_FILE}*"): + os.remove(f) + @pytest.fixture(autouse=True) def disable_installed_check() -> Generator[None]: @@ -85,16 +96,11 @@ async def test_async_enable_logging( hass: HomeAssistant, caplog: pytest.LogCaptureFixture ) -> None: """Test to ensure logging is migrated to the queue handlers.""" - config_log_file_pattern = get_test_config_dir("home-assistant.log*") - arg_log_file_pattern = "test.log*" # Ensure we start with a clean slate - for f in glob.glob(arg_log_file_pattern): - os.remove(f) - for f in glob.glob(config_log_file_pattern): - os.remove(f) - assert len(glob.glob(config_log_file_pattern)) == 0 - assert len(glob.glob(arg_log_file_pattern)) == 0 + cleanup_log_files() + assert len(glob.glob(CONFIG_LOG_FILE)) == 0 + assert len(glob.glob(ARG_LOG_FILE)) == 0 with ( patch("logging.getLogger"), @@ -108,7 +114,7 @@ async def test_async_enable_logging( ): await bootstrap.async_enable_logging(hass) mock_async_activate_log_queue_handler.assert_called_once() - assert len(glob.glob(config_log_file_pattern)) > 0 + assert len(glob.glob(CONFIG_LOG_FILE)) > 0 mock_async_activate_log_queue_handler.reset_mock() await bootstrap.async_enable_logging( @@ -117,14 +123,61 @@ async def test_async_enable_logging( log_file="test.log", ) mock_async_activate_log_queue_handler.assert_called_once() - assert len(glob.glob(arg_log_file_pattern)) > 0 + assert len(glob.glob(ARG_LOG_FILE)) > 0 assert "Error rolling over log file" in caplog.text - for f in glob.glob(arg_log_file_pattern): - os.remove(f) - for f in glob.glob(config_log_file_pattern): - os.remove(f) + cleanup_log_files() + + +async def test_async_enable_logging_supervisor( + hass: HomeAssistant, caplog: pytest.LogCaptureFixture +) -> None: + """Test to ensure the default log file is not created on Supervisor installations.""" + + # Ensure we start with a clean slate + cleanup_log_files() + assert len(glob.glob(CONFIG_LOG_FILE)) == 0 + assert len(glob.glob(ARG_LOG_FILE)) == 0 + + with ( + patch.dict(os.environ, {"SUPERVISOR": "1"}), + patch( + "homeassistant.bootstrap.async_activate_log_queue_handler" + ) as mock_async_activate_log_queue_handler, + patch("logging.getLogger"), + ): + await bootstrap.async_enable_logging(hass) + assert len(glob.glob(CONFIG_LOG_FILE)) == 0 + mock_async_activate_log_queue_handler.assert_called_once() + mock_async_activate_log_queue_handler.reset_mock() + + # Check that if the log file exists, it is renamed + def write_log_file(): + with open( + get_test_config_dir("home-assistant.log"), "w", encoding="utf8" + ) as f: + f.write("test") + + await hass.async_add_executor_job(write_log_file) + assert len(glob.glob(CONFIG_LOG_FILE)) == 1 + assert len(glob.glob(f"{CONFIG_LOG_FILE}.old")) == 0 + await bootstrap.async_enable_logging(hass) + assert len(glob.glob(CONFIG_LOG_FILE)) == 0 + assert len(glob.glob(f"{CONFIG_LOG_FILE}.old")) == 1 + mock_async_activate_log_queue_handler.assert_called_once() + mock_async_activate_log_queue_handler.reset_mock() + + await bootstrap.async_enable_logging( + hass, + log_rotate_days=5, + log_file="test.log", + ) + mock_async_activate_log_queue_handler.assert_called_once() + # Even on Supervisor, the log file should be created if it is explicitly specified + assert len(glob.glob(ARG_LOG_FILE)) > 0 + + cleanup_log_files() async def test_load_hassio(hass: HomeAssistant) -> None: