From 34301b1de4a0889ee123b48b7487854c461b1296 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 21 Jan 2025 18:49:56 +0100 Subject: [PATCH 1/5] Extend backup API with file name field Allow to specify a backup file name when creating a backup. This allows for user friendly backup file names. If none is specified, the current behavior remains (backup file name is the backup slug). --- supervisor/api/backups.py | 2 ++ supervisor/backups/manager.py | 13 ++++++++++--- tests/backups/test_manager.py | 22 ++++++++++++++++++++++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/supervisor/api/backups.py b/supervisor/api/backups.py index b37671bd9e3..b1bb445fe21 100644 --- a/supervisor/api/backups.py +++ b/supervisor/api/backups.py @@ -26,6 +26,7 @@ ATTR_DATE, ATTR_DAYS_UNTIL_STALE, ATTR_EXTRA, + ATTR_FILENAME, ATTR_FOLDERS, ATTR_HOMEASSISTANT, ATTR_HOMEASSISTANT_EXCLUDE_DATABASE, @@ -98,6 +99,7 @@ def _ensure_list(item: Any) -> list: SCHEMA_BACKUP_FULL = vol.Schema( { vol.Optional(ATTR_NAME): str, + vol.Optional(ATTR_FILENAME): str, vol.Optional(ATTR_PASSWORD): vol.Maybe(str), vol.Optional(ATTR_COMPRESSED): vol.Maybe(vol.Boolean()), vol.Optional(ATTR_LOCATION): vol.All( diff --git a/supervisor/backups/manager.py b/supervisor/backups/manager.py index 1ebd477004e..83d0514e5b3 100644 --- a/supervisor/backups/manager.py +++ b/supervisor/backups/manager.py @@ -184,6 +184,7 @@ def _list_backup_files(self, path: Path) -> Iterable[Path]: def _create_backup( self, name: str, + filename: str | None, sys_type: BackupType, password: str | None, compressed: bool = True, @@ -196,7 +197,11 @@ def _create_backup( """ date_str = utcnow().isoformat() slug = create_slug(name, date_str) - tar_file = Path(self._get_base_path(location), f"{slug}.tar") + + if filename: + tar_file = Path(self._get_base_path(location), Path(filename).name) + else: + tar_file = Path(self._get_base_path(location), f"{slug}.tar") # init object backup = Backup(self.coresys, tar_file, slug, self._get_location_name(location)) @@ -482,6 +487,7 @@ async def _do_backup( async def do_backup_full( self, name: str = "", + filename: str | None = None, *, password: str | None = None, compressed: bool = True, @@ -500,7 +506,7 @@ async def do_backup_full( ) backup = self._create_backup( - name, BackupType.FULL, password, compressed, location, extra + name, filename, BackupType.FULL, password, compressed, location, extra ) _LOGGER.info("Creating new full backup with slug %s", backup.slug) @@ -526,6 +532,7 @@ async def do_backup_full( async def do_backup_partial( self, name: str = "", + filename: str | None = None, *, addons: list[str] | None = None, folders: list[str] | None = None, @@ -558,7 +565,7 @@ async def do_backup_partial( _LOGGER.error("Nothing to create backup for") backup = self._create_backup( - name, BackupType.PARTIAL, password, compressed, location, extra + name, filename, BackupType.PARTIAL, password, compressed, location, extra ) _LOGGER.info("Creating new partial backup with slug %s", backup.slug) diff --git a/tests/backups/test_manager.py b/tests/backups/test_manager.py index 52dcefa8537..0e940d13823 100644 --- a/tests/backups/test_manager.py +++ b/tests/backups/test_manager.py @@ -73,6 +73,28 @@ async def test_do_backup_full(coresys: CoreSys, backup_mock, install_addon_ssh): assert coresys.core.state == CoreState.RUNNING +@pytest.mark.parametrize( + ("filename", "filename_expected"), + [("../my file.tar", "/data/backup/my file.tar"), (None, "/data/backup/{}.tar")], +) +async def test_do_backup_full_with_filename( + coresys: CoreSys, filename: str, filename_expected: str, backup_mock +): + """Test creating Backup with a specific file name.""" + coresys.core.state = CoreState.RUNNING + coresys.hardware.disk.get_disk_free_space = lambda x: 5000 + + manager = BackupManager(coresys) + + # backup_mock fixture causes Backup() to be a MagicMock + await manager.do_backup_full(filename=filename) + + slug = backup_mock.call_args[0][2] + assert str(backup_mock.call_args[0][1]) == filename_expected.format(slug) + + assert coresys.core.state == CoreState.RUNNING + + async def test_do_backup_full_uncompressed( coresys: CoreSys, backup_mock, install_addon_ssh ): From 8c14810e6d46d78a22e3a5d02dea2010c4c28439 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Wed, 22 Jan 2025 21:22:53 +0100 Subject: [PATCH 2/5] Check passed file name using regex --- supervisor/api/backups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/supervisor/api/backups.py b/supervisor/api/backups.py index b1bb445fe21..1b1b8e38e61 100644 --- a/supervisor/api/backups.py +++ b/supervisor/api/backups.py @@ -99,7 +99,7 @@ def _ensure_list(item: Any) -> list: SCHEMA_BACKUP_FULL = vol.Schema( { vol.Optional(ATTR_NAME): str, - vol.Optional(ATTR_FILENAME): str, + vol.Optional(ATTR_FILENAME): vol.Match(RE_BACKUP_FILENAME), vol.Optional(ATTR_PASSWORD): vol.Maybe(str), vol.Optional(ATTR_COMPRESSED): vol.Maybe(vol.Boolean()), vol.Optional(ATTR_LOCATION): vol.All( From bcb8f2a7044ad39e14e302acd93d30f196bcef59 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Wed, 22 Jan 2025 21:28:48 +0100 Subject: [PATCH 3/5] Use custom filename on download only if backup file name is backup slug --- supervisor/api/backups.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/supervisor/api/backups.py b/supervisor/api/backups.py index 1b1b8e38e61..6c826c32f18 100644 --- a/supervisor/api/backups.py +++ b/supervisor/api/backups.py @@ -460,10 +460,14 @@ async def download(self, request: web.Request): raise APIError(f"Backup {backup.slug} is not in location {location}") _LOGGER.info("Downloading backup %s", backup.slug) - response = web.FileResponse(backup.all_locations[location]) + filename = backup.all_locations[location] + response = web.FileResponse(filename) response.content_type = CONTENT_TYPE_TAR + + if filename == f"{backup.slug}.tar": + filename = f"{RE_SLUGIFY_NAME.sub('_', backup.name)}.tar" response.headers[CONTENT_DISPOSITION] = ( - f"attachment; filename={RE_SLUGIFY_NAME.sub('_', backup.name)}.tar" + f"attachment; filename={filename}" ) return response From e295348f0675e499f3abe0498c8ebb3b75ca3d9b Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Wed, 22 Jan 2025 21:32:59 +0100 Subject: [PATCH 4/5] ruff format --- supervisor/api/backups.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/supervisor/api/backups.py b/supervisor/api/backups.py index 6c826c32f18..4f0331280f8 100644 --- a/supervisor/api/backups.py +++ b/supervisor/api/backups.py @@ -466,9 +466,7 @@ async def download(self, request: web.Request): if filename == f"{backup.slug}.tar": filename = f"{RE_SLUGIFY_NAME.sub('_', backup.name)}.tar" - response.headers[CONTENT_DISPOSITION] = ( - f"attachment; filename={filename}" - ) + response.headers[CONTENT_DISPOSITION] = f"attachment; filename={filename}" return response @api_process From 997b2817a8a832405930ebce2ded7b3222eb5538 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Wed, 22 Jan 2025 22:15:18 +0100 Subject: [PATCH 5/5] Remove path from location for download file name --- supervisor/api/backups.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/supervisor/api/backups.py b/supervisor/api/backups.py index 4f0331280f8..0e1e4623d23 100644 --- a/supervisor/api/backups.py +++ b/supervisor/api/backups.py @@ -464,9 +464,12 @@ async def download(self, request: web.Request): response = web.FileResponse(filename) response.content_type = CONTENT_TYPE_TAR - if filename == f"{backup.slug}.tar": - filename = f"{RE_SLUGIFY_NAME.sub('_', backup.name)}.tar" - response.headers[CONTENT_DISPOSITION] = f"attachment; filename={filename}" + download_filename = filename.name + if download_filename == f"{backup.slug}.tar": + download_filename = f"{RE_SLUGIFY_NAME.sub('_', backup.name)}.tar" + response.headers[CONTENT_DISPOSITION] = ( + f"attachment; filename={download_filename}" + ) return response @api_process