From beaeea25b374df1e60e3b69dc891eff6e5e92834 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 15 Jun 2026 11:56:04 -0700 Subject: [PATCH] Fix for the "restore" action sometimes failing to find a database dump that was dumped with a default port (#1322). --- NEWS | 2 + borgmatic/hooks/data_source/mariadb.py | 64 +++++++++++---- borgmatic/hooks/data_source/mongodb.py | 64 +++++++++++---- borgmatic/hooks/data_source/mysql.py | 64 +++++++++++---- borgmatic/hooks/data_source/postgresql.py | 64 +++++++++++---- tests/unit/hooks/data_source/test_mariadb.py | 80 +++++++++++++++++++ tests/unit/hooks/data_source/test_mongodb.py | 80 +++++++++++++++++++ tests/unit/hooks/data_source/test_mysql.py | 80 +++++++++++++++++++ .../unit/hooks/data_source/test_postgresql.py | 80 +++++++++++++++++++ 9 files changed, 510 insertions(+), 68 deletions(-) diff --git a/NEWS b/NEWS index 52ac2b04..c1bc591d 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,8 @@ action. Borg >= 1.4.5 and < 2 only. * #1319: Fix the ZFS hook's overzealous unmounting of snapshot paths when a source dataset is at "/". + * #1322: Fix for the "restore" action sometimes failing to find a database dump that was dumped + with a default port. * Fix a bug in which the "compact" action does not pass a compact threshold of zero to Borg. 2.1.6 diff --git a/borgmatic/hooks/data_source/mariadb.py b/borgmatic/hooks/data_source/mariadb.py index f83bcc8e..a2c36e8d 100644 --- a/borgmatic/hooks/data_source/mariadb.py +++ b/borgmatic/hooks/data_source/mariadb.py @@ -422,7 +422,7 @@ def make_data_source_dump_patterns( port=None, container=None, label=None, -): # pragma: no cover +): ''' Given a sequence of configurations dicts, a configuration dict, the borgmatic runtime directory, and a database name to match, return the corresponding glob patterns to match the database dump @@ -431,24 +431,54 @@ def make_data_source_dump_patterns( borgmatic_source_directory = borgmatic.config.paths.get_borgmatic_source_directory(config) return ( - dump.make_data_source_dump_filename( - make_dump_path('borgmatic'), name, hostname, port, container, label + *( + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), name, hostname, port, container, label + ), + dump.make_data_source_dump_filename( + make_dump_path(borgmatic_runtime_directory), + name, + hostname, + port, + container, + label, + ), + dump.make_data_source_dump_filename( + make_dump_path(borgmatic_source_directory), + name, + hostname, + port, + container, + label, + ), ), - dump.make_data_source_dump_filename( - make_dump_path(borgmatic_runtime_directory), - name, - hostname, - port, - container, - label, + *( + ( + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), + name, + hostname, + port=None, + container=container, + label=label, + ), + ) + if port == get_default_port(databases, config) + else () ), - dump.make_data_source_dump_filename( - make_dump_path(borgmatic_source_directory), - name, - hostname, - port, - container, - label, + *( + ( + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), + name, + hostname, + port=get_default_port(databases, config), + container=container, + label=label, + ), + ) + if port is None + else () ), ) diff --git a/borgmatic/hooks/data_source/mongodb.py b/borgmatic/hooks/data_source/mongodb.py index f95deb78..5b4adbb5 100644 --- a/borgmatic/hooks/data_source/mongodb.py +++ b/borgmatic/hooks/data_source/mongodb.py @@ -216,7 +216,7 @@ def make_data_source_dump_patterns( port=None, container=None, label=None, -): # pragma: no cover +): ''' Given a sequence of configurations dicts, a configuration dict, the borgmatic runtime directory, and a database name to match, return the corresponding glob patterns to match the database dump @@ -225,24 +225,54 @@ def make_data_source_dump_patterns( borgmatic_source_directory = borgmatic.config.paths.get_borgmatic_source_directory(config) return ( - dump.make_data_source_dump_filename( - make_dump_path('borgmatic'), name, hostname, port, container, label + *( + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), name, hostname, port, container, label + ), + dump.make_data_source_dump_filename( + make_dump_path(borgmatic_runtime_directory), + name, + hostname, + port, + container, + label, + ), + dump.make_data_source_dump_filename( + make_dump_path(borgmatic_source_directory), + name, + hostname, + port, + container, + label, + ), ), - dump.make_data_source_dump_filename( - make_dump_path(borgmatic_runtime_directory), - name, - hostname, - port, - container, - label, + *( + ( + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), + name, + hostname, + port=None, + container=container, + label=label, + ), + ) + if port == get_default_port(databases, config) + else () ), - dump.make_data_source_dump_filename( - make_dump_path(borgmatic_source_directory), - name, - hostname, - port, - container, - label, + *( + ( + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), + name, + hostname, + port=get_default_port(databases, config), + container=container, + label=label, + ), + ) + if port is None + else () ), ) diff --git a/borgmatic/hooks/data_source/mysql.py b/borgmatic/hooks/data_source/mysql.py index af17eb62..1a81c4c8 100644 --- a/borgmatic/hooks/data_source/mysql.py +++ b/borgmatic/hooks/data_source/mysql.py @@ -343,7 +343,7 @@ def make_data_source_dump_patterns( port=None, container=None, label=None, -): # pragma: no cover +): ''' Given a sequence of configurations dicts, a configuration dict, the borgmatic runtime directory, and a database name to match, return the corresponding glob patterns to match the database dump @@ -352,24 +352,54 @@ def make_data_source_dump_patterns( borgmatic_source_directory = borgmatic.config.paths.get_borgmatic_source_directory(config) return ( - dump.make_data_source_dump_filename( - make_dump_path('borgmatic'), name, hostname, port, container, label + *( + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), name, hostname, port, container, label + ), + dump.make_data_source_dump_filename( + make_dump_path(borgmatic_runtime_directory), + name, + hostname, + port, + container, + label, + ), + dump.make_data_source_dump_filename( + make_dump_path(borgmatic_source_directory), + name, + hostname, + port, + container, + label, + ), ), - dump.make_data_source_dump_filename( - make_dump_path(borgmatic_runtime_directory), - name, - hostname, - port, - container, - label, + *( + ( + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), + name, + hostname, + port=None, + container=container, + label=label, + ), + ) + if port == get_default_port(databases, config) + else () ), - dump.make_data_source_dump_filename( - make_dump_path(borgmatic_source_directory), - name, - hostname, - port, - container, - label, + *( + ( + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), + name, + hostname, + port=get_default_port(databases, config), + container=container, + label=label, + ), + ) + if port is None + else () ), ) diff --git a/borgmatic/hooks/data_source/postgresql.py b/borgmatic/hooks/data_source/postgresql.py index 885bd5f4..2c6f06fb 100644 --- a/borgmatic/hooks/data_source/postgresql.py +++ b/borgmatic/hooks/data_source/postgresql.py @@ -310,7 +310,7 @@ def make_data_source_dump_patterns( port=None, container=None, label=None, -): # pragma: no cover +): ''' Given a sequence of configurations dicts, a configuration dict, the borgmatic runtime directory, and a database name to match, return the corresponding glob patterns to match the database dump @@ -319,24 +319,54 @@ def make_data_source_dump_patterns( borgmatic_source_directory = borgmatic.config.paths.get_borgmatic_source_directory(config) return ( - dump.make_data_source_dump_filename( - make_dump_path('borgmatic'), name, hostname, port, container, label + *( + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), name, hostname, port, container, label + ), + dump.make_data_source_dump_filename( + make_dump_path(borgmatic_runtime_directory), + name, + hostname, + port, + container, + label, + ), + dump.make_data_source_dump_filename( + make_dump_path(borgmatic_source_directory), + name, + hostname, + port, + container, + label, + ), ), - dump.make_data_source_dump_filename( - make_dump_path(borgmatic_runtime_directory), - name, - hostname, - port, - container, - label, + *( + ( + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), + name, + hostname, + port=None, + container=container, + label=label, + ), + ) + if port == get_default_port(databases, config) + else () ), - dump.make_data_source_dump_filename( - make_dump_path(borgmatic_source_directory), - name, - hostname, - port, - container, - label, + *( + ( + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), + name, + hostname, + port=get_default_port(databases, config), + container=container, + label=label, + ), + ) + if port is None + else () ), ) diff --git a/tests/unit/hooks/data_source/test_mariadb.py b/tests/unit/hooks/data_source/test_mariadb.py index c0af5485..ec04bd0c 100644 --- a/tests/unit/hooks/data_source/test_mariadb.py +++ b/tests/unit/hooks/data_source/test_mariadb.py @@ -1426,6 +1426,86 @@ def test_execute_dump_command_with_dry_run_skips_mariadb_dump(): ) +def test_make_data_source_dump_patterns_with_no_port_adds_pattern_with_default_port(): + flexmock(module.borgmatic.config.paths).should_receive( + 'get_borgmatic_source_directory' + ).and_return('.borgmatic') + flexmock(module).should_receive('make_dump_path').replace_with(lambda path: path) + flexmock(module.dump).should_receive('make_data_source_dump_filename').replace_with( + lambda dump_path, name, hostname, port, container, label: '/'.join( + (dump_path, f'{hostname}:{port}' if port else hostname, name) + ) + ) + flexmock(module).should_receive('get_default_port').and_return(9999) + + assert module.make_data_source_dump_patterns( + databases=flexmock(), + config=flexmock(), + borgmatic_runtime_directory='run', + name='db', + hostname='host', + port=None, + ) == ( + 'borgmatic/host/db', + 'run/host/db', + '.borgmatic/host/db', + 'borgmatic/host:9999/db', + ) + + +def test_make_data_source_dump_patterns_with_default_port_adds_pattern_with_no_port(): + flexmock(module.borgmatic.config.paths).should_receive( + 'get_borgmatic_source_directory' + ).and_return('.borgmatic') + flexmock(module).should_receive('make_dump_path').replace_with(lambda path: path) + flexmock(module.dump).should_receive('make_data_source_dump_filename').replace_with( + lambda dump_path, name, hostname, port, container, label: '/'.join( + (dump_path, f'{hostname}:{port}' if port else hostname, name) + ) + ) + flexmock(module).should_receive('get_default_port').and_return(9999) + + assert module.make_data_source_dump_patterns( + databases=flexmock(), + config=flexmock(), + borgmatic_runtime_directory='run', + name='db', + hostname='host', + port=9999, + ) == ( + 'borgmatic/host:9999/db', + 'run/host:9999/db', + '.borgmatic/host:9999/db', + 'borgmatic/host/db', + ) + + +def test_make_data_source_dump_patterns_with_non_default_port_adds_no_extra_patterns(): + flexmock(module.borgmatic.config.paths).should_receive( + 'get_borgmatic_source_directory' + ).and_return('.borgmatic') + flexmock(module).should_receive('make_dump_path').replace_with(lambda path: path) + flexmock(module.dump).should_receive('make_data_source_dump_filename').replace_with( + lambda dump_path, name, hostname, port, container, label: '/'.join( + (dump_path, f'{hostname}:{port}' if port else hostname, name) + ) + ) + flexmock(module).should_receive('get_default_port').and_return(9999) + + assert module.make_data_source_dump_patterns( + databases=flexmock(), + config=flexmock(), + borgmatic_runtime_directory='run', + name='db', + hostname='host', + port=1234, + ) == ( + 'borgmatic/host:1234/db', + 'run/host:1234/db', + '.borgmatic/host:1234/db', + ) + + def test_restore_data_source_dump_runs_mariadb_to_restore(): hook_config = [{'name': 'foo'}, {'name': 'bar'}] extract_process = flexmock(stdout=flexmock()) diff --git a/tests/unit/hooks/data_source/test_mongodb.py b/tests/unit/hooks/data_source/test_mongodb.py index 19faa092..851adce1 100644 --- a/tests/unit/hooks/data_source/test_mongodb.py +++ b/tests/unit/hooks/data_source/test_mongodb.py @@ -378,6 +378,86 @@ def test_build_dump_command_with_username_injection_attack_gets_escaped(): assert "'bob; naughty-command'" in command +def test_make_data_source_dump_patterns_with_no_port_adds_pattern_with_default_port(): + flexmock(module.borgmatic.config.paths).should_receive( + 'get_borgmatic_source_directory' + ).and_return('.borgmatic') + flexmock(module).should_receive('make_dump_path').replace_with(lambda path: path) + flexmock(module.dump).should_receive('make_data_source_dump_filename').replace_with( + lambda dump_path, name, hostname, port, container, label: '/'.join( + (dump_path, f'{hostname}:{port}' if port else hostname, name) + ) + ) + flexmock(module).should_receive('get_default_port').and_return(9999) + + assert module.make_data_source_dump_patterns( + databases=flexmock(), + config=flexmock(), + borgmatic_runtime_directory='run', + name='db', + hostname='host', + port=None, + ) == ( + 'borgmatic/host/db', + 'run/host/db', + '.borgmatic/host/db', + 'borgmatic/host:9999/db', + ) + + +def test_make_data_source_dump_patterns_with_default_port_adds_pattern_with_no_port(): + flexmock(module.borgmatic.config.paths).should_receive( + 'get_borgmatic_source_directory' + ).and_return('.borgmatic') + flexmock(module).should_receive('make_dump_path').replace_with(lambda path: path) + flexmock(module.dump).should_receive('make_data_source_dump_filename').replace_with( + lambda dump_path, name, hostname, port, container, label: '/'.join( + (dump_path, f'{hostname}:{port}' if port else hostname, name) + ) + ) + flexmock(module).should_receive('get_default_port').and_return(9999) + + assert module.make_data_source_dump_patterns( + databases=flexmock(), + config=flexmock(), + borgmatic_runtime_directory='run', + name='db', + hostname='host', + port=9999, + ) == ( + 'borgmatic/host:9999/db', + 'run/host:9999/db', + '.borgmatic/host:9999/db', + 'borgmatic/host/db', + ) + + +def test_make_data_source_dump_patterns_with_non_default_port_adds_no_extra_patterns(): + flexmock(module.borgmatic.config.paths).should_receive( + 'get_borgmatic_source_directory' + ).and_return('.borgmatic') + flexmock(module).should_receive('make_dump_path').replace_with(lambda path: path) + flexmock(module.dump).should_receive('make_data_source_dump_filename').replace_with( + lambda dump_path, name, hostname, port, container, label: '/'.join( + (dump_path, f'{hostname}:{port}' if port else hostname, name) + ) + ) + flexmock(module).should_receive('get_default_port').and_return(9999) + + assert module.make_data_source_dump_patterns( + databases=flexmock(), + config=flexmock(), + borgmatic_runtime_directory='run', + name='db', + hostname='host', + port=1234, + ) == ( + 'borgmatic/host:1234/db', + 'run/host:1234/db', + '.borgmatic/host:1234/db', + ) + + def test_restore_data_source_dump_runs_mongorestore(): hook_config = [{'name': 'foo', 'schemas': None}, {'name': 'bar'}] extract_process = flexmock(stdout=flexmock()) diff --git a/tests/unit/hooks/data_source/test_mysql.py b/tests/unit/hooks/data_source/test_mysql.py index e32ae653..b2f9a639 100644 --- a/tests/unit/hooks/data_source/test_mysql.py +++ b/tests/unit/hooks/data_source/test_mysql.py @@ -1239,6 +1239,86 @@ def test_execute_dump_command_with_dry_run_skips_mysqldump(): ) +def test_make_data_source_dump_patterns_with_no_port_adds_pattern_with_default_port(): + flexmock(module.borgmatic.config.paths).should_receive( + 'get_borgmatic_source_directory' + ).and_return('.borgmatic') + flexmock(module).should_receive('make_dump_path').replace_with(lambda path: path) + flexmock(module.dump).should_receive('make_data_source_dump_filename').replace_with( + lambda dump_path, name, hostname, port, container, label: '/'.join( + (dump_path, f'{hostname}:{port}' if port else hostname, name) + ) + ) + flexmock(module).should_receive('get_default_port').and_return(9999) + + assert module.make_data_source_dump_patterns( + databases=flexmock(), + config=flexmock(), + borgmatic_runtime_directory='run', + name='db', + hostname='host', + port=None, + ) == ( + 'borgmatic/host/db', + 'run/host/db', + '.borgmatic/host/db', + 'borgmatic/host:9999/db', + ) + + +def test_make_data_source_dump_patterns_with_default_port_adds_pattern_with_no_port(): + flexmock(module.borgmatic.config.paths).should_receive( + 'get_borgmatic_source_directory' + ).and_return('.borgmatic') + flexmock(module).should_receive('make_dump_path').replace_with(lambda path: path) + flexmock(module.dump).should_receive('make_data_source_dump_filename').replace_with( + lambda dump_path, name, hostname, port, container, label: '/'.join( + (dump_path, f'{hostname}:{port}' if port else hostname, name) + ) + ) + flexmock(module).should_receive('get_default_port').and_return(9999) + + assert module.make_data_source_dump_patterns( + databases=flexmock(), + config=flexmock(), + borgmatic_runtime_directory='run', + name='db', + hostname='host', + port=9999, + ) == ( + 'borgmatic/host:9999/db', + 'run/host:9999/db', + '.borgmatic/host:9999/db', + 'borgmatic/host/db', + ) + + +def test_make_data_source_dump_patterns_with_non_default_port_adds_no_extra_patterns(): + flexmock(module.borgmatic.config.paths).should_receive( + 'get_borgmatic_source_directory' + ).and_return('.borgmatic') + flexmock(module).should_receive('make_dump_path').replace_with(lambda path: path) + flexmock(module.dump).should_receive('make_data_source_dump_filename').replace_with( + lambda dump_path, name, hostname, port, container, label: '/'.join( + (dump_path, f'{hostname}:{port}' if port else hostname, name) + ) + ) + flexmock(module).should_receive('get_default_port').and_return(9999) + + assert module.make_data_source_dump_patterns( + databases=flexmock(), + config=flexmock(), + borgmatic_runtime_directory='run', + name='db', + hostname='host', + port=1234, + ) == ( + 'borgmatic/host:1234/db', + 'run/host:1234/db', + '.borgmatic/host:1234/db', + ) + + def test_restore_data_source_dump_runs_mysql_to_restore(): hook_config = [{'name': 'foo'}, {'name': 'bar'}] extract_process = flexmock(stdout=flexmock()) diff --git a/tests/unit/hooks/data_source/test_postgresql.py b/tests/unit/hooks/data_source/test_postgresql.py index 44bc2099..17d7034a 100644 --- a/tests/unit/hooks/data_source/test_postgresql.py +++ b/tests/unit/hooks/data_source/test_postgresql.py @@ -979,6 +979,86 @@ def test_dump_data_sources_runs_non_default_pg_dump(): ) == [process] +def test_make_data_source_dump_patterns_with_no_port_adds_pattern_with_default_port(): + flexmock(module.borgmatic.config.paths).should_receive( + 'get_borgmatic_source_directory' + ).and_return('.borgmatic') + flexmock(module).should_receive('make_dump_path').replace_with(lambda path: path) + flexmock(module.dump).should_receive('make_data_source_dump_filename').replace_with( + lambda dump_path, name, hostname, port, container, label: '/'.join( + (dump_path, f'{hostname}:{port}' if port else hostname, name) + ) + ) + flexmock(module).should_receive('get_default_port').and_return(9999) + + assert module.make_data_source_dump_patterns( + databases=flexmock(), + config=flexmock(), + borgmatic_runtime_directory='run', + name='db', + hostname='host', + port=None, + ) == ( + 'borgmatic/host/db', + 'run/host/db', + '.borgmatic/host/db', + 'borgmatic/host:9999/db', + ) + + +def test_make_data_source_dump_patterns_with_default_port_adds_pattern_with_no_port(): + flexmock(module.borgmatic.config.paths).should_receive( + 'get_borgmatic_source_directory' + ).and_return('.borgmatic') + flexmock(module).should_receive('make_dump_path').replace_with(lambda path: path) + flexmock(module.dump).should_receive('make_data_source_dump_filename').replace_with( + lambda dump_path, name, hostname, port, container, label: '/'.join( + (dump_path, f'{hostname}:{port}' if port else hostname, name) + ) + ) + flexmock(module).should_receive('get_default_port').and_return(9999) + + assert module.make_data_source_dump_patterns( + databases=flexmock(), + config=flexmock(), + borgmatic_runtime_directory='run', + name='db', + hostname='host', + port=9999, + ) == ( + 'borgmatic/host:9999/db', + 'run/host:9999/db', + '.borgmatic/host:9999/db', + 'borgmatic/host/db', + ) + + +def test_make_data_source_dump_patterns_with_non_default_port_adds_no_extra_patterns(): + flexmock(module.borgmatic.config.paths).should_receive( + 'get_borgmatic_source_directory' + ).and_return('.borgmatic') + flexmock(module).should_receive('make_dump_path').replace_with(lambda path: path) + flexmock(module.dump).should_receive('make_data_source_dump_filename').replace_with( + lambda dump_path, name, hostname, port, container, label: '/'.join( + (dump_path, f'{hostname}:{port}' if port else hostname, name) + ) + ) + flexmock(module).should_receive('get_default_port').and_return(9999) + + assert module.make_data_source_dump_patterns( + databases=flexmock(), + config=flexmock(), + borgmatic_runtime_directory='run', + name='db', + hostname='host', + port=1234, + ) == ( + 'borgmatic/host:1234/db', + 'run/host:1234/db', + '.borgmatic/host:1234/db', + ) + + def test_restore_data_source_dump_runs_pg_restore(): hook_config = [{'name': 'foo', 'schemas': None}, {'name': 'bar'}] extract_process = flexmock(stdout=flexmock())