Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve hassio backup create and restore parameter checks #134434

Merged
merged 2 commits into from
Jan 2, 2025
Merged

Improve hassio backup create and restore parameter checks #134434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
76ed3ff
Improve hassio backup create and restore parameter checks
emontnemery Jan 2, 2025
399de27
Merge branch 'dev' into hassio_backup_improve_checks
emontnemery Jan 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions homeassistant/components/hassio/backup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,10 @@ async def async_create_backup(
password: str | None,
) -> tuple[NewBackup, asyncio.Task[WrittenBackup]]:
"""Create a backup."""
if not include_homeassistant and include_database:
raise HomeAssistantError(
"Cannot create a backup with database but without Home Assistant"
)
manager = self._hass.data[DATA_MANAGER]

include_addons_set: supervisor_backups.AddonSet | set[str] | None = None
Expand Down Expand Up @@ -360,8 +364,16 @@ async def async_restore_backup(
restore_homeassistant: bool,
) -> None:
"""Restore a backup."""
if restore_homeassistant and not restore_database:
raise HomeAssistantError("Cannot restore Home Assistant without database")
manager = self._hass.data[DATA_MANAGER]
# The backup manager has already checked that the backup exists so we don't need to
# check that here.
backup = await manager.backup_agents[agent_id].async_get_backup(backup_id)
if (
backup
and restore_homeassistant
and restore_database != backup.database_included
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this implements what you write here:

It's not possible to restore database from a backup which doesn't have database or not restore the database from a backup which has database

and from what I understand what Supervisor is capable of today, correct?

IMHO, not restoring the data should be possible, but there is the concern what if this is a downgrade of Home Assistant Core, and the (currently newer) database is not compatible? 🤔 The Core will complain on startup, so it is "safe", but the outcome might not be what the user expect (since he presumably wants to continue with the current database).

When no database is part of the backup, the user can already run into this situation, so we are a bit inconsistent in what we allow today 😢

):
raise HomeAssistantError("Restore database must match backup")
if not restore_homeassistant and restore_database:
raise HomeAssistantError("Cannot restore database without Home Assistant")
restore_addons_set = set(restore_addons) if restore_addons else None
Expand All @@ -371,7 +383,6 @@ async def async_restore_backup(
else None
)

manager = self._hass.data[DATA_MANAGER]
restore_location: str | None
if manager.backup_agents[agent_id].domain != DOMAIN:
# Download the backup to the supervisor. Supervisor will clean up the backup
Expand Down
98 changes: 89 additions & 9 deletions tests/components/hassio/test_backup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,51 @@
)


TEST_BACKUP_4 = supervisor_backups.Backup(
compressed=False,
content=supervisor_backups.BackupContent(
addons=["ssl"],
folders=["share"],
homeassistant=True,
),
date=datetime.fromisoformat("1970-01-01T00:00:00Z"),
location=None,
locations={None},
name="Test",
protected=False,
size=1.0,
size_bytes=1048576,
slug="abc123",
type=supervisor_backups.BackupType.PARTIAL,
)
TEST_BACKUP_DETAILS_4 = supervisor_backups.BackupComplete(
addons=[
supervisor_backups.BackupAddon(
name="Terminal & SSH",
size=0.0,
slug="core_ssh",
version="9.14.0",
)
],
compressed=TEST_BACKUP.compressed,
date=TEST_BACKUP.date,
extra=None,
folders=["share"],
homeassistant_exclude_database=True,
homeassistant="2024.12.0",
location=TEST_BACKUP.location,
locations=TEST_BACKUP.locations,
name=TEST_BACKUP.name,
protected=TEST_BACKUP.protected,
repositories=[],
size=TEST_BACKUP.size,
size_bytes=TEST_BACKUP.size_bytes,
slug=TEST_BACKUP.slug,
supervisor_version="2024.11.2",
type=TEST_BACKUP.type,
)


@pytest.fixture(autouse=True)
def fixture_supervisor_environ() -> Generator[None]:
"""Mock os environ for supervisor."""
Expand Down Expand Up @@ -661,8 +706,17 @@ async def test_agents_notify_on_mount_added_removed(
replace(DEFAULT_BACKUP_OPTIONS, folders={"media", "share"}),
),
(
{"include_folders": ["media"], "include_homeassistant": False},
replace(DEFAULT_BACKUP_OPTIONS, folders={"media"}, homeassistant=False),
{
"include_folders": ["media"],
"include_database": False,
"include_homeassistant": False,
},
replace(
DEFAULT_BACKUP_OPTIONS,
folders={"media"},
homeassistant=False,
homeassistant_exclude_database=True,
),
),
],
)
Expand Down Expand Up @@ -813,16 +867,30 @@ async def test_reader_writer_create_remote_backup(

@pytest.mark.usefixtures("hassio_client", "setup_integration")
@pytest.mark.parametrize(
("extra_generate_options"),
("extra_generate_options", "expected_error"),
[
{"include_homeassistant": False},
(
{"include_homeassistant": False},
{
"code": "home_assistant_error",
"message": "Cannot create a backup with database but without Home Assistant",
},
),
(
{"include_homeassistant": False, "include_database": False},
{
"code": "unknown_error",
"message": "Unknown error",
},
),
],
)
async def test_reader_writer_create_wrong_parameters(
hass: HomeAssistant,
hass_ws_client: WebSocketGenerator,
supervisor_client: AsyncMock,
extra_generate_options: dict[str, Any],
expected_error: dict[str, str],
) -> None:
"""Test generating a backup."""
client = await hass_ws_client(hass)
Expand Down Expand Up @@ -860,7 +928,7 @@ async def test_reader_writer_create_wrong_parameters(

response = await client.receive_json()
assert not response["success"]
assert response["error"] == {"code": "unknown_error", "message": "Unknown error"}
assert response["error"] == expected_error

supervisor_client.backups.partial_backup.assert_not_called()

Expand Down Expand Up @@ -1069,30 +1137,42 @@ async def test_reader_writer_restore_error(


@pytest.mark.parametrize(
("parameters", "expected_error"),
("backup", "backup_details", "parameters", "expected_error"),
[
(
TEST_BACKUP,
TEST_BACKUP_DETAILS,
{"restore_database": False},
"Cannot restore Home Assistant without database",
"Restore database must match backup",
),
(
TEST_BACKUP,
TEST_BACKUP_DETAILS,
{"restore_homeassistant": False},
"Cannot restore database without Home Assistant",
),
(
TEST_BACKUP_4,
TEST_BACKUP_DETAILS_4,
{"restore_homeassistant": True, "restore_database": True},
"Restore database must match backup",
),
],
)
@pytest.mark.usefixtures("hassio_client", "setup_integration")
async def test_reader_writer_restore_wrong_parameters(
hass: HomeAssistant,
hass_ws_client: WebSocketGenerator,
supervisor_client: AsyncMock,
backup: supervisor_backups.Backup,
backup_details: supervisor_backups.BackupComplete,
parameters: dict[str, Any],
expected_error: str,
) -> None:
"""Test trigger restore."""
client = await hass_ws_client(hass)
supervisor_client.backups.list.return_value = [TEST_BACKUP]
supervisor_client.backups.backup_info.return_value = TEST_BACKUP_DETAILS
supervisor_client.backups.list.return_value = [backup]
supervisor_client.backups.backup_info.return_value = backup_details

default_parameters = {
"type": "backup/restore",
Expand Down
Loading