From f558cb31563dcf303ca5eebc469a8e462f2ccbe6 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Mon, 12 Jun 2023 21:54:39 +0530 Subject: [PATCH 01/18] feat: allow restoring to different port/host/username --- borgmatic/config/schema.yaml | 24 ++++++++++++++++++++++++ borgmatic/hooks/postgresql.py | 11 +++++++---- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 903c043..50abc3f 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -763,10 +763,21 @@ properties: Database hostname to connect to. Defaults to connecting via local Unix socket. example: database.example.org + restore_hostname: + type: string + description: | + Database hostname to restore to. Defaults to + the "hostname" option. + example: database.example.org port: type: integer description: Port to connect to. Defaults to 5432. example: 5433 + restore_port: + type: integer + description: Port to restore to. Defaults to the + "port" option. + example: 5433 username: type: string description: | @@ -775,6 +786,12 @@ properties: You probably want to specify the "postgres" superuser here when the database name is "all". example: dbuser + restore_username: + type: string + description: | + Username with which to restore to the database. + Defaults to the "username" option. + example: dbuser password: type: string description: | @@ -784,6 +801,13 @@ properties: without a password or you create a ~/.pgpass file. example: trustsome1 + restore_password: + type: string + description: | + Password with which to connect to the database that + is being restored to. Defaults to the "password" + option. + example: trustsome1 format: type: string enum: ['plain', 'custom', 'directory', 'tar'] diff --git a/borgmatic/hooks/postgresql.py b/borgmatic/hooks/postgresql.py index 3325391..dab1e83 100644 --- a/borgmatic/hooks/postgresql.py +++ b/borgmatic/hooks/postgresql.py @@ -217,10 +217,10 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, analyze_command = ( tuple(psql_command) + ('--no-password', '--no-psqlrc', '--quiet') - + (('--host', database['hostname']) if 'hostname' in database else ()) - + (('--port', str(database['port'])) if 'port' in database else ()) - + (('--username', database['username']) if 'username' in database else ()) - + (('--dbname', database['name']) if not all_databases else ()) + + (('--host', database.get('restore_hostname', database.get('hostname', ())))) + + (('--port', str(database.get('restore_port', database.get('port', ())))) + + (('--username', database.get('restore_username', database.get('username', ())))) + + (('--dbname', database['name']) if not all_databases else ())) + (tuple(database['analyze_options'].split(' ')) if 'analyze_options' in database else ()) + ('--command', 'ANALYZE') ) @@ -245,6 +245,9 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, extra_environment = make_extra_environment(database) + if 'restore_password' in database: + extra_environment['PGPASSWORD'] = database['restore_password'] + logger.debug(f"{log_prefix}: Restoring PostgreSQL database {database['name']}{dry_run_label}") if dry_run: return From 8e8e64d920a8d4ad7c31dd12b7da361b616987f4 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Tue, 13 Jun 2023 23:42:50 +0530 Subject: [PATCH 02/18] add no-owner and refactor --- borgmatic/config/schema.yaml | 21 +++++++++++++++++---- borgmatic/hooks/postgresql.py | 25 +++++++++++++------------ 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 50abc3f..20fe3f9 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -789,7 +789,7 @@ properties: restore_username: type: string description: | - Username with which to restore to the database. + Username with which to restore the database. Defaults to the "username" option. example: dbuser password: @@ -804,10 +804,23 @@ properties: restore_password: type: string description: | - Password with which to connect to the database that - is being restored to. Defaults to the "password" - option. + Password with which to connect to the restore + database. Defaults to the "password" option. example: trustsome1 + no_owner: + type: boolean + description: | + Do not output commands to set ownership of + objects to match the original database. By + default, pg_dump and pg_restore issue ALTER + OWNER or SET SESSION AUTHORIZATION statements + to set ownership of created schema elements. + These statements will fail unless the initial + connection to the database is made by a superuser + (in which case they will execute as though wrapped + in SECURITY DEFINER functions). When --no-owner + is used, neither the ALTER OWNER nor SET SESSION + AUTHORIZATION statements will be emitted. format: type: string enum: ['plain', 'custom', 'directory', 'tar'] diff --git a/borgmatic/hooks/postgresql.py b/borgmatic/hooks/postgresql.py index dab1e83..95ec4ac 100644 --- a/borgmatic/hooks/postgresql.py +++ b/borgmatic/hooks/postgresql.py @@ -23,13 +23,15 @@ def make_dump_path(location_config): # pragma: no cover ) -def make_extra_environment(database): +def make_extra_environment(database, restore=False): ''' Make the extra_environment dict from the given database configuration. ''' extra = dict() if 'password' in database: extra['PGPASSWORD'] = database['password'] + if restore and 'restore_password' in database: + extra['PGPASSWORD'] = database['restore_password'] extra['PGSSLMODE'] = database.get('ssl_mode', 'disable') if 'ssl_cert' in database: extra['PGSSLCERT'] = database['ssl_cert'] @@ -135,6 +137,7 @@ def dump_databases(databases, log_prefix, location_config, dry_run): + (('--host', database['hostname']) if 'hostname' in database else ()) + (('--port', str(database['port'])) if 'port' in database else ()) + (('--username', database['username']) if 'username' in database else ()) + + (('--no-owner',) if database['no_owner'] else ()) + (('--format', dump_format) if dump_format else ()) + (('--file', dump_filename) if dump_format == 'directory' else ()) + (tuple(database['options'].split(' ')) if 'options' in database else ()) @@ -217,10 +220,10 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, analyze_command = ( tuple(psql_command) + ('--no-password', '--no-psqlrc', '--quiet') - + (('--host', database.get('restore_hostname', database.get('hostname', ())))) - + (('--port', str(database.get('restore_port', database.get('port', ())))) - + (('--username', database.get('restore_username', database.get('username', ())))) - + (('--dbname', database['name']) if not all_databases else ())) + + (('--host', database.get('restore_hostname', database.get('hostname'))) if 'hostname' in database else ()) + + (('--port', str(database.get('restore_port', database.get('port')))) if 'port' in database else ()) + + (('--username', database.get('restore_username', database.get('username'))) if 'username' in database else ()) + + (('--dbname', database['name']) if not all_databases else ()) + (tuple(database['analyze_options'].split(' ')) if 'analyze_options' in database else ()) + ('--command', 'ANALYZE') ) @@ -231,9 +234,10 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, + ('--no-password',) + (('--no-psqlrc',) if use_psql_command else ('--if-exists', '--exit-on-error', '--clean')) + (('--dbname', database['name']) if not all_databases else ()) - + (('--host', database['hostname']) if 'hostname' in database else ()) - + (('--port', str(database['port'])) if 'port' in database else ()) - + (('--username', database['username']) if 'username' in database else ()) + + (('--host', database.get('restore_hostname', database.get('hostname'))) if 'hostname' in database or 'restore_hostname' in database else ()) + + (('--port', str(database.get('restore_port', database.get('port')))) if 'port' in database or 'restore_port' in database else ()) + + (('--username', database.get('restore_username', database.get('username'))) if 'username' in database or 'restore_username' in database else ()) + + (('--no-owner',) if database['no_owner'] else ()) + (tuple(database['restore_options'].split(' ')) if 'restore_options' in database else ()) + (() if extract_process else (dump_filename,)) + tuple( @@ -243,10 +247,7 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, ) ) - extra_environment = make_extra_environment(database) - - if 'restore_password' in database: - extra_environment['PGPASSWORD'] = database['restore_password'] + extra_environment = make_extra_environment(database, restore=True) logger.debug(f"{log_prefix}: Restoring PostgreSQL database {database['name']}{dry_run_label}") if dry_run: From 230cf6adc45a3f15cae54da11d934fe2560f8c45 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Wed, 14 Jun 2023 00:11:19 +0530 Subject: [PATCH 03/18] support command line args for hostname port username password --- borgmatic/actions/restore.py | 17 ++++++++++++++++- borgmatic/commands/arguments.py | 15 +++++++++++++++ borgmatic/hooks/postgresql.py | 30 ++++++++++++++++++++---------- 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index ded83f4..a701aa0 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -68,15 +68,21 @@ def restore_single_database( archive_name, hook_name, database, + connection_params, ): # pragma: no cover ''' - Given (among other things) an archive name, a database hook name, and a configured database + Given (among other things) an archive name, a database hook name, the hostname, + port, username and password as connection params, and a configured database configuration dict, restore that database from the archive. ''' logger.info( f'{repository.get("label", repository["path"])}: Restoring database {database["name"]}' ) + logger.info( + f'hostname port username password for database {database["name"]}' + ) + dump_pattern = borgmatic.hooks.dispatch.call_hooks( 'make_database_dump_pattern', hooks, @@ -113,6 +119,7 @@ def restore_single_database( location, global_arguments.dry_run, extract_process, + connection_params, ) @@ -308,6 +315,13 @@ def run_restore( hooks, archive_database_names, hook_name, database_name ) + connection_params = { + 'hostname': restore_arguments.hostname, + 'port': restore_arguments.port, + 'username': restore_arguments.username, + 'password': restore_arguments.password, + } + if not found_database: remaining_restore_names.setdefault(found_hook_name or hook_name, []).append( database_name @@ -327,6 +341,7 @@ def run_restore( archive_name, found_hook_name or hook_name, dict(found_database, **{'schemas': restore_arguments.schemas}), + connection_params, ) # For any database that weren't found via exact matches in the hooks configuration, try to diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 6039e4f..02f2fc3 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -720,6 +720,21 @@ def make_parsers(): dest='schemas', help='Names of schemas to restore from the database, defaults to all schemas. Schemas are only supported for PostgreSQL and MongoDB databases', ) + restore_group.add_argument( + '--hostname', + help='Database hostname to restore to. Defaults to the "restore_hostname" option in borgmatic\'s configuration', + ) + restore_group.add_argument( + '--port', help='Port to restore to. Defaults to the "restore_port" option in borgmatic\'s configuration' + ) + restore_group.add_argument( + '--username', + help='Username with which to connect to the database. Defaults to the "restore_username" option in borgmatic\'s configuration', + ) + restore_group.add_argument( + '--password', + help='Password with which to connect to the restore database. Defaults to the "restore_password" option in borgmatic\'s configuration', + ) restore_group.add_argument( '-h', '--help', action='help', help='Show this help message and exit' ) diff --git a/borgmatic/hooks/postgresql.py b/borgmatic/hooks/postgresql.py index 95ec4ac..89ade66 100644 --- a/borgmatic/hooks/postgresql.py +++ b/borgmatic/hooks/postgresql.py @@ -22,8 +22,7 @@ def make_dump_path(location_config): # pragma: no cover location_config.get('borgmatic_source_directory'), 'postgresql_databases' ) - -def make_extra_environment(database, restore=False): +def make_extra_environment(database, restore=False, connection_params=None): ''' Make the extra_environment dict from the given database configuration. ''' @@ -32,6 +31,8 @@ def make_extra_environment(database, restore=False): extra['PGPASSWORD'] = database['password'] if restore and 'restore_password' in database: extra['PGPASSWORD'] = database['restore_password'] + if connection_params is not None and connection_params.get('password'): + extra['PGPASSWORD'] = connection_params['password'] extra['PGSSLMODE'] = database.get('ssl_mode', 'disable') if 'ssl_cert' in database: extra['PGSSLCERT'] = database['ssl_cert'] @@ -195,7 +196,7 @@ def make_database_dump_pattern( return dump.make_database_dump_filename(make_dump_path(location_config), name, hostname='*') -def restore_database_dump(database_config, log_prefix, location_config, dry_run, extract_process): +def restore_database_dump(database_config, log_prefix, location_config, dry_run, extract_process, connection_params): ''' Restore the given PostgreSQL database from an extract stream. The database is supplied as a one-element sequence containing a dict describing the database, as per the configuration schema. @@ -205,6 +206,9 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, If the extract process is None, then restore the dump from the filesystem rather than from an extract stream. + + Use the given connection parameters to connect to the database. The connection parameters are + hostname, port, username, and password. ''' dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else '' @@ -212,6 +216,12 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, raise ValueError('The database configuration value is invalid') database = database_config[0] + + hostname = connection_params['hostname'] or database.get('restore_hostname', database.get('hostname')) + port = str(connection_params['port'] or database.get('restore_port', database.get('port'))) + username = connection_params['username'] or database.get('restore_username', database.get('username')) + password = connection_params['password'] or database.get('restore_password', database.get('password')) + all_databases = bool(database['name'] == 'all') dump_filename = dump.make_database_dump_filename( make_dump_path(location_config), database['name'], database.get('hostname') @@ -220,9 +230,9 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, analyze_command = ( tuple(psql_command) + ('--no-password', '--no-psqlrc', '--quiet') - + (('--host', database.get('restore_hostname', database.get('hostname'))) if 'hostname' in database else ()) - + (('--port', str(database.get('restore_port', database.get('port')))) if 'port' in database else ()) - + (('--username', database.get('restore_username', database.get('username'))) if 'username' in database else ()) + + (('--host', hostname) if 'hostname' in database or 'restore_hostname' in database or 'hostname' in connection_params else ()) + + (('--port', port) if 'port' in database or 'restore_port' in database or 'port' in connection_params else ()) + + (('--username', username) if 'username' in database or 'restore_username' in database or 'username' in connection_params else ()) + (('--dbname', database['name']) if not all_databases else ()) + (tuple(database['analyze_options'].split(' ')) if 'analyze_options' in database else ()) + ('--command', 'ANALYZE') @@ -234,9 +244,9 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, + ('--no-password',) + (('--no-psqlrc',) if use_psql_command else ('--if-exists', '--exit-on-error', '--clean')) + (('--dbname', database['name']) if not all_databases else ()) - + (('--host', database.get('restore_hostname', database.get('hostname'))) if 'hostname' in database or 'restore_hostname' in database else ()) - + (('--port', str(database.get('restore_port', database.get('port')))) if 'port' in database or 'restore_port' in database else ()) - + (('--username', database.get('restore_username', database.get('username'))) if 'username' in database or 'restore_username' in database else ()) + + (('--host', hostname) if 'hostname' in database or 'restore_hostname' in database or 'hostname' in connection_params else ()) + + (('--port', port) if 'port' in database or 'restore_port' in database or 'port' in connection_params else ()) + + (('--username', username) if 'username' in database or 'restore_username' in database or 'username' in connection_params else ()) + (('--no-owner',) if database['no_owner'] else ()) + (tuple(database['restore_options'].split(' ')) if 'restore_options' in database else ()) + (() if extract_process else (dump_filename,)) @@ -247,7 +257,7 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, ) ) - extra_environment = make_extra_environment(database, restore=True) + extra_environment = make_extra_environment(database, restore=True, connection_params=connection_params) logger.debug(f"{log_prefix}: Restoring PostgreSQL database {database['name']}{dry_run_label}") if dry_run: From 67f4d43aece67321affa6ec45bd0fffd15d40748 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Thu, 15 Jun 2023 01:37:18 +0530 Subject: [PATCH 04/18] witten review --- borgmatic/actions/restore.py | 4 ---- borgmatic/config/schema.yaml | 6 +----- borgmatic/hooks/postgresql.py | 31 +++++++++++++++++-------------- 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index a701aa0..6d8eb4e 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -79,10 +79,6 @@ def restore_single_database( f'{repository.get("label", repository["path"])}: Restoring database {database["name"]}' ) - logger.info( - f'hostname port username password for database {database["name"]}' - ) - dump_pattern = borgmatic.hooks.dispatch.call_hooks( 'make_database_dump_pattern', hooks, diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 20fe3f9..87de0c6 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -816,11 +816,7 @@ properties: OWNER or SET SESSION AUTHORIZATION statements to set ownership of created schema elements. These statements will fail unless the initial - connection to the database is made by a superuser - (in which case they will execute as though wrapped - in SECURITY DEFINER functions). When --no-owner - is used, neither the ALTER OWNER nor SET SESSION - AUTHORIZATION statements will be emitted. + connection to the database is made by a superuser. format: type: string enum: ['plain', 'custom', 'directory', 'tar'] diff --git a/borgmatic/hooks/postgresql.py b/borgmatic/hooks/postgresql.py index 89ade66..d1c92f8 100644 --- a/borgmatic/hooks/postgresql.py +++ b/borgmatic/hooks/postgresql.py @@ -22,17 +22,20 @@ def make_dump_path(location_config): # pragma: no cover location_config.get('borgmatic_source_directory'), 'postgresql_databases' ) -def make_extra_environment(database, restore=False, connection_params=None): +def make_extra_environment(database, restore_connection_params=None): ''' Make the extra_environment dict from the given database configuration. + If restore connection params are given, this is for a restore operation. ''' extra = dict() if 'password' in database: extra['PGPASSWORD'] = database['password'] - if restore and 'restore_password' in database: - extra['PGPASSWORD'] = database['restore_password'] - if connection_params is not None and connection_params.get('password'): - extra['PGPASSWORD'] = connection_params['password'] + + try: + extra['PGPASSWORD'] = restore_connection_params.get('password') or database['restore_password'] + except (AttributeError, KeyError): + pass + extra['PGSSLMODE'] = database.get('ssl_mode', 'disable') if 'ssl_cert' in database: extra['PGSSLCERT'] = database['ssl_cert'] @@ -138,7 +141,7 @@ def dump_databases(databases, log_prefix, location_config, dry_run): + (('--host', database['hostname']) if 'hostname' in database else ()) + (('--port', str(database['port'])) if 'port' in database else ()) + (('--username', database['username']) if 'username' in database else ()) - + (('--no-owner',) if database['no_owner'] else ()) + + (('--no-owner',) if database.get('no_owner', False) else ()) + (('--format', dump_format) if dump_format else ()) + (('--file', dump_filename) if dump_format == 'directory' else ()) + (tuple(database['options'].split(' ')) if 'options' in database else ()) @@ -230,9 +233,9 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, analyze_command = ( tuple(psql_command) + ('--no-password', '--no-psqlrc', '--quiet') - + (('--host', hostname) if 'hostname' in database or 'restore_hostname' in database or 'hostname' in connection_params else ()) - + (('--port', port) if 'port' in database or 'restore_port' in database or 'port' in connection_params else ()) - + (('--username', username) if 'username' in database or 'restore_username' in database or 'username' in connection_params else ()) + + (('--host', hostname) if hostname else ()) + + (('--port', port) if port else ()) + + (('--username', username) if username else ()) + (('--dbname', database['name']) if not all_databases else ()) + (tuple(database['analyze_options'].split(' ')) if 'analyze_options' in database else ()) + ('--command', 'ANALYZE') @@ -244,10 +247,10 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, + ('--no-password',) + (('--no-psqlrc',) if use_psql_command else ('--if-exists', '--exit-on-error', '--clean')) + (('--dbname', database['name']) if not all_databases else ()) - + (('--host', hostname) if 'hostname' in database or 'restore_hostname' in database or 'hostname' in connection_params else ()) - + (('--port', port) if 'port' in database or 'restore_port' in database or 'port' in connection_params else ()) - + (('--username', username) if 'username' in database or 'restore_username' in database or 'username' in connection_params else ()) - + (('--no-owner',) if database['no_owner'] else ()) + + (('--host', hostname) if hostname else ()) + + (('--port', port) if port else ()) + + (('--username', username) if username else ()) + + (('--no-owner',) if database.get('no_owner', False) else ()) + (tuple(database['restore_options'].split(' ')) if 'restore_options' in database else ()) + (() if extract_process else (dump_filename,)) + tuple( @@ -257,7 +260,7 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, ) ) - extra_environment = make_extra_environment(database, restore=True, connection_params=connection_params) + extra_environment = make_extra_environment(database, restore_connection_params=connection_params) logger.debug(f"{log_prefix}: Restoring PostgreSQL database {database['name']}{dry_run_label}") if dry_run: From 205e5b152466f0315d3761a99002c7f4b3c2d8db Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Thu, 15 Jun 2023 01:47:46 +0530 Subject: [PATCH 05/18] mysql support --- borgmatic/hooks/mysql.py | 18 ++++++++++++------ borgmatic/hooks/postgresql.py | 5 +---- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/borgmatic/hooks/mysql.py b/borgmatic/hooks/mysql.py index 793b78b..b5218dc 100644 --- a/borgmatic/hooks/mysql.py +++ b/borgmatic/hooks/mysql.py @@ -185,7 +185,7 @@ def make_database_dump_pattern( return dump.make_database_dump_filename(make_dump_path(location_config), name, hostname='*') -def restore_database_dump(database_config, log_prefix, location_config, dry_run, extract_process): +def restore_database_dump(database_config, log_prefix, location_config, dry_run, extract_process, connection_params): ''' Restore the given MySQL/MariaDB database from an extract stream. The database is supplied as a one-element sequence containing a dict describing the database, as per the configuration schema. @@ -199,15 +199,21 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, raise ValueError('The database configuration value is invalid') database = database_config[0] + + hostname = connection_params['hostname'] or database.get('restore_hostname', database.get('hostname')) + port = str(connection_params['port'] or database.get('restore_port', database.get('port'))) + username = connection_params['username'] or database.get('restore_username', database.get('username')) + password = connection_params['password'] or database.get('restore_password', database.get('password')) + restore_command = ( ('mysql', '--batch') + (tuple(database['restore_options'].split(' ')) if 'restore_options' in database else ()) - + (('--host', database['hostname']) if 'hostname' in database else ()) - + (('--port', str(database['port'])) if 'port' in database else ()) - + (('--protocol', 'tcp') if 'hostname' in database or 'port' in database else ()) - + (('--user', database['username']) if 'username' in database else ()) + + (('--host', database['hostname']) if hostname else ()) + + (('--port', str(database['port'])) if port else ()) + + (('--protocol', 'tcp') if hostname or port else ()) + + (('--user', database['username']) if username else ()) ) - extra_environment = {'MYSQL_PWD': database['password']} if 'password' in database else None + extra_environment = {'MYSQL_PWD': password} if password else None logger.debug(f"{log_prefix}: Restoring MySQL database {database['name']}{dry_run_label}") if dry_run: diff --git a/borgmatic/hooks/postgresql.py b/borgmatic/hooks/postgresql.py index d1c92f8..c90ae63 100644 --- a/borgmatic/hooks/postgresql.py +++ b/borgmatic/hooks/postgresql.py @@ -28,11 +28,9 @@ def make_extra_environment(database, restore_connection_params=None): If restore connection params are given, this is for a restore operation. ''' extra = dict() - if 'password' in database: - extra['PGPASSWORD'] = database['password'] try: - extra['PGPASSWORD'] = restore_connection_params.get('password') or database['restore_password'] + extra['PGPASSWORD'] = restore_connection_params.get('password') or database['restore_password'] or database['password'] except (AttributeError, KeyError): pass @@ -223,7 +221,6 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, hostname = connection_params['hostname'] or database.get('restore_hostname', database.get('hostname')) port = str(connection_params['port'] or database.get('restore_port', database.get('port'))) username = connection_params['username'] or database.get('restore_username', database.get('username')) - password = connection_params['password'] or database.get('restore_password', database.get('password')) all_databases = bool(database['name'] == 'all') dump_filename = dump.make_database_dump_filename( From a9386b7a8789ace90b9bb6a4d0c6c29a4aee5d29 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Thu, 15 Jun 2023 02:18:24 +0530 Subject: [PATCH 06/18] add mongodb support, and sqlite restore path (config option only) --- borgmatic/config/schema.yaml | 52 ++++++++++++++++++++++++++++++++++++ borgmatic/hooks/mongodb.py | 27 +++++++++++-------- 2 files changed, 68 insertions(+), 11 deletions(-) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 87de0c6..77da6ef 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -952,16 +952,33 @@ properties: Database hostname to connect to. Defaults to connecting via local Unix socket. example: database.example.org + restore_hostname: + type: string + description: | + Database hostname to restore to. Defaults to + the "hostname" option. + example: database.example.org port: type: integer description: Port to connect to. Defaults to 3306. example: 3307 + restore_port: + type: integer + description: Port to restore to. Defaults to the + "port" option. + example: 5433 username: type: string description: | Username with which to connect to the database. Defaults to the username of the current user. example: dbuser + restore_username: + type: string + description: | + Username with which to restore the database. + Defaults to the "username" option. + example: dbuser password: type: string description: | @@ -970,6 +987,12 @@ properties: configured to trust the configured username without a password. example: trustsome1 + restore_password: + type: string + description: | + Password with which to connect to the restore + database. Defaults to the "password" option. + example: trustsome1 format: type: string enum: ['sql'] @@ -1047,6 +1070,12 @@ properties: read_special and one_file_system (see above) to support dump and restore streaming. example: /var/lib/sqlite/users.db + restore_path: + type: string + description: | + Path to the SQLite database file to restore to. + Defaults to the "path" option. + example: /var/lib/sqlite/users.db mongodb_databases: type: array items: @@ -1069,22 +1098,45 @@ properties: Database hostname to connect to. Defaults to connecting to localhost. example: database.example.org + restore_hostname: + type: string + description: | + Database hostname to restore to. Defaults to + the "hostname" option. + example: database.example.org port: type: integer description: Port to connect to. Defaults to 27017. example: 27018 + restore_port: + type: integer + description: Port to restore to. Defaults to the + "port" option. + example: 5433 username: type: string description: | Username with which to connect to the database. Skip it if no authentication is needed. example: dbuser + restore_username: + type: string + description: | + Username with which to restore the database. + Defaults to the "username" option. + example: dbuser password: type: string description: | Password with which to connect to the database. Skip it if no authentication is needed. example: trustsome1 + restore_password: + type: string + description: | + Password with which to connect to the restore + database. Defaults to the "password" option. + example: trustsome1 authentication_database: type: string description: | diff --git a/borgmatic/hooks/mongodb.py b/borgmatic/hooks/mongodb.py index 781e5f2..1cbd98f 100644 --- a/borgmatic/hooks/mongodb.py +++ b/borgmatic/hooks/mongodb.py @@ -102,7 +102,7 @@ def make_database_dump_pattern( return dump.make_database_dump_filename(make_dump_path(location_config), name, hostname='*') -def restore_database_dump(database_config, log_prefix, location_config, dry_run, extract_process): +def restore_database_dump(database_config, log_prefix, location_config, dry_run, extract_process, connection_params): ''' Restore the given MongoDB database from an extract stream. The database is supplied as a one-element sequence containing a dict describing the database, as per the configuration schema. @@ -122,7 +122,7 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, dump_filename = dump.make_database_dump_filename( make_dump_path(location_config), database['name'], database.get('hostname') ) - restore_command = build_restore_command(extract_process, database, dump_filename) + restore_command = build_restore_command(extract_process, database, dump_filename, connection_params) logger.debug(f"{log_prefix}: Restoring MongoDB database {database['name']}{dry_run_label}") if dry_run: @@ -138,10 +138,15 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, ) -def build_restore_command(extract_process, database, dump_filename): +def build_restore_command(extract_process, database, dump_filename, connection_params): ''' Return the mongorestore command from a single database configuration. ''' + hostname = connection_params['hostname'] or database.get('restore_hostname', database.get('hostname')) + port = str(connection_params['port'] or database.get('restore_port', database.get('port'))) + username = connection_params['username'] or database.get('restore_username', database.get('username')) + password = connection_params['password'] or database.get('restore_password', database.get('password')) + command = ['mongorestore'] if extract_process: command.append('--archive') @@ -149,14 +154,14 @@ def build_restore_command(extract_process, database, dump_filename): command.extend(('--dir', dump_filename)) if database['name'] != 'all': command.extend(('--drop', '--db', database['name'])) - if 'hostname' in database: - command.extend(('--host', database['hostname'])) - if 'port' in database: - command.extend(('--port', str(database['port']))) - if 'username' in database: - command.extend(('--username', database['username'])) - if 'password' in database: - command.extend(('--password', database['password'])) + if hostname: + command.extend(('--host', hostname)) + if port: + command.extend(('--port', str(port))) + if username: + command.extend(('--username', username)) + if password: + command.extend(('--password', password)) if 'authentication_database' in database: command.extend(('--authenticationDatabase', database['authentication_database'])) if 'restore_options' in database: From b7423c488e9df2174cbbbbaebdf5a9c6f319b9c6 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Thu, 15 Jun 2023 22:54:06 +0530 Subject: [PATCH 07/18] refactor password assignment logic --- borgmatic/actions/restore.py | 2 +- borgmatic/hooks/postgresql.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 6d8eb4e..532c587 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -72,7 +72,7 @@ def restore_single_database( ): # pragma: no cover ''' Given (among other things) an archive name, a database hook name, the hostname, - port, username and password as connection params, and a configured database + port, username and password as connection params, and a configured database configuration dict, restore that database from the archive. ''' logger.info( diff --git a/borgmatic/hooks/postgresql.py b/borgmatic/hooks/postgresql.py index c90ae63..8672028 100644 --- a/borgmatic/hooks/postgresql.py +++ b/borgmatic/hooks/postgresql.py @@ -30,7 +30,10 @@ def make_extra_environment(database, restore_connection_params=None): extra = dict() try: - extra['PGPASSWORD'] = restore_connection_params.get('password') or database['restore_password'] or database['password'] + if restore_connection_params: + extra['PGPASSWORD'] = restore_connection_params.get('password') or database.get('restore_password', database['password']) + else: + extra['PGPASSWORD'] = database['password'] except (AttributeError, KeyError): pass From 62b6f1329923020da8ffdbe0dc6109f5358189bb Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Thu, 15 Jun 2023 23:02:09 +0530 Subject: [PATCH 08/18] add restore-path support for sqlite --- borgmatic/actions/restore.py | 1 + borgmatic/hooks/sqlite.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 532c587..156f376 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -316,6 +316,7 @@ def run_restore( 'port': restore_arguments.port, 'username': restore_arguments.username, 'password': restore_arguments.password, + 'restore_path': restore_arguments.restore_path, } if not found_database: diff --git a/borgmatic/hooks/sqlite.py b/borgmatic/hooks/sqlite.py index d9f105d..4e4e0be 100644 --- a/borgmatic/hooks/sqlite.py +++ b/borgmatic/hooks/sqlite.py @@ -85,7 +85,7 @@ def make_database_dump_pattern( return dump.make_database_dump_filename(make_dump_path(location_config), name) -def restore_database_dump(database_config, log_prefix, location_config, dry_run, extract_process): +def restore_database_dump(database_config, log_prefix, location_config, dry_run, extract_process, connection_params): ''' Restore the given SQLite3 database from an extract stream. The database is supplied as a one-element sequence containing a dict describing the database, as per the configuration schema. @@ -98,7 +98,7 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, if len(database_config) != 1: raise ValueError('The database configuration value is invalid') - database_path = database_config[0]['path'] + database_path = connection_params['restore_path'] or database_config[0].get('restore_path', database_config[0].get('path')) logger.debug(f'{log_prefix}: Restoring SQLite database at {database_path}{dry_run_label}') if dry_run: From 82d851d8911b0e77e6705d2196908e343b40383d Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Thu, 15 Jun 2023 23:05:53 +0530 Subject: [PATCH 09/18] add argument for restore path --- borgmatic/commands/arguments.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 02f2fc3..61db198 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -735,6 +735,10 @@ def make_parsers(): '--password', help='Password with which to connect to the restore database. Defaults to the "restore_password" option in borgmatic\'s configuration', ) + restore_group.add_argument( + '--restore-path', + help='Path to restore SQLite database dumps to. Defaults to the "restore_path" option in borgmatic\'s configuration', + ) restore_group.add_argument( '-h', '--help', action='help', help='Show this help message and exit' ) From 8389851f2f907164713a621240b4e0f6e2e66663 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Thu, 15 Jun 2023 23:34:50 +0530 Subject: [PATCH 10/18] fix bug where port becomes truthy when none is converted to str --- borgmatic/hooks/mongodb.py | 2 +- borgmatic/hooks/mysql.py | 2 +- borgmatic/hooks/postgresql.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/borgmatic/hooks/mongodb.py b/borgmatic/hooks/mongodb.py index 1cbd98f..f8432b1 100644 --- a/borgmatic/hooks/mongodb.py +++ b/borgmatic/hooks/mongodb.py @@ -143,7 +143,7 @@ def build_restore_command(extract_process, database, dump_filename, connection_p Return the mongorestore command from a single database configuration. ''' hostname = connection_params['hostname'] or database.get('restore_hostname', database.get('hostname')) - port = str(connection_params['port'] or database.get('restore_port', database.get('port'))) + port = str(connection_params['port'] or database.get('restore_port', database.get('port', ''))) username = connection_params['username'] or database.get('restore_username', database.get('username')) password = connection_params['password'] or database.get('restore_password', database.get('password')) diff --git a/borgmatic/hooks/mysql.py b/borgmatic/hooks/mysql.py index b5218dc..f64f16f 100644 --- a/borgmatic/hooks/mysql.py +++ b/borgmatic/hooks/mysql.py @@ -201,7 +201,7 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, database = database_config[0] hostname = connection_params['hostname'] or database.get('restore_hostname', database.get('hostname')) - port = str(connection_params['port'] or database.get('restore_port', database.get('port'))) + port = str(connection_params['port'] or database.get('restore_port', database.get('port', ''))) username = connection_params['username'] or database.get('restore_username', database.get('username')) password = connection_params['password'] or database.get('restore_password', database.get('password')) diff --git a/borgmatic/hooks/postgresql.py b/borgmatic/hooks/postgresql.py index 8672028..08b0019 100644 --- a/borgmatic/hooks/postgresql.py +++ b/borgmatic/hooks/postgresql.py @@ -222,7 +222,7 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, database = database_config[0] hostname = connection_params['hostname'] or database.get('restore_hostname', database.get('hostname')) - port = str(connection_params['port'] or database.get('restore_port', database.get('port'))) + port = str(connection_params['port'] or database.get('restore_port', database.get('port', ''))) username = connection_params['username'] or database.get('restore_username', database.get('username')) all_databases = bool(database['name'] == 'all') From 89602d1614b80bd14e6ab314bd6f84ba49c352bf Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Fri, 16 Jun 2023 15:14:00 +0530 Subject: [PATCH 11/18] pass all existing tests (and formatting) --- borgmatic/actions/restore.py | 16 ++-- borgmatic/commands/arguments.py | 3 +- borgmatic/config/schema.yaml | 5 +- borgmatic/hooks/mongodb.py | 20 +++-- borgmatic/hooks/mysql.py | 16 +++- borgmatic/hooks/postgresql.py | 23 +++-- borgmatic/hooks/sqlite.py | 8 +- tests/unit/actions/test_restore.py | 48 +++++++++- tests/unit/hooks/test_mongodb.py | 108 +++++++++++++++++++++-- tests/unit/hooks/test_mysql.py | 72 +++++++++++++-- tests/unit/hooks/test_postgresql.py | 132 +++++++++++++++++++++++++--- tests/unit/hooks/test_sqlite.py | 21 ++++- 12 files changed, 411 insertions(+), 61 deletions(-) diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 156f376..d44a2ca 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -304,6 +304,13 @@ def run_restore( restore_names = find_databases_to_restore(restore_arguments.databases, archive_database_names) found_names = set() remaining_restore_names = {} + connection_params = { + 'hostname': restore_arguments.hostname, + 'port': restore_arguments.port, + 'username': restore_arguments.username, + 'password': restore_arguments.password, + 'restore_path': restore_arguments.restore_path, + } for hook_name, database_names in restore_names.items(): for database_name in database_names: @@ -311,14 +318,6 @@ def run_restore( hooks, archive_database_names, hook_name, database_name ) - connection_params = { - 'hostname': restore_arguments.hostname, - 'port': restore_arguments.port, - 'username': restore_arguments.username, - 'password': restore_arguments.password, - 'restore_path': restore_arguments.restore_path, - } - if not found_database: remaining_restore_names.setdefault(found_hook_name or hook_name, []).append( database_name @@ -368,6 +367,7 @@ def run_restore( archive_name, found_hook_name or hook_name, dict(database, **{'schemas': restore_arguments.schemas}), + connection_params, ) borgmatic.hooks.dispatch.call_hooks_even_if_unconfigured( diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 61db198..241e6ef 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -725,7 +725,8 @@ def make_parsers(): help='Database hostname to restore to. Defaults to the "restore_hostname" option in borgmatic\'s configuration', ) restore_group.add_argument( - '--port', help='Port to restore to. Defaults to the "restore_port" option in borgmatic\'s configuration' + '--port', + help='Port to restore to. Defaults to the "restore_port" option in borgmatic\'s configuration', ) restore_group.add_argument( '--username', diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 77da6ef..7d079ff 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -816,7 +816,8 @@ properties: OWNER or SET SESSION AUTHORIZATION statements to set ownership of created schema elements. These statements will fail unless the initial - connection to the database is made by a superuser. + connection to the database is made by a + superuser. format: type: string enum: ['plain', 'custom', 'directory', 'tar'] @@ -1103,7 +1104,7 @@ properties: description: | Database hostname to restore to. Defaults to the "hostname" option. - example: database.example.org + example: database.example.org port: type: integer description: Port to connect to. Defaults to 27017. diff --git a/borgmatic/hooks/mongodb.py b/borgmatic/hooks/mongodb.py index f8432b1..f889926 100644 --- a/borgmatic/hooks/mongodb.py +++ b/borgmatic/hooks/mongodb.py @@ -102,7 +102,9 @@ def make_database_dump_pattern( return dump.make_database_dump_filename(make_dump_path(location_config), name, hostname='*') -def restore_database_dump(database_config, log_prefix, location_config, dry_run, extract_process, connection_params): +def restore_database_dump( + database_config, log_prefix, location_config, dry_run, extract_process, connection_params +): ''' Restore the given MongoDB database from an extract stream. The database is supplied as a one-element sequence containing a dict describing the database, as per the configuration schema. @@ -122,7 +124,9 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, dump_filename = dump.make_database_dump_filename( make_dump_path(location_config), database['name'], database.get('hostname') ) - restore_command = build_restore_command(extract_process, database, dump_filename, connection_params) + restore_command = build_restore_command( + extract_process, database, dump_filename, connection_params + ) logger.debug(f"{log_prefix}: Restoring MongoDB database {database['name']}{dry_run_label}") if dry_run: @@ -142,10 +146,16 @@ def build_restore_command(extract_process, database, dump_filename, connection_p ''' Return the mongorestore command from a single database configuration. ''' - hostname = connection_params['hostname'] or database.get('restore_hostname', database.get('hostname')) + hostname = connection_params['hostname'] or database.get( + 'restore_hostname', database.get('hostname') + ) port = str(connection_params['port'] or database.get('restore_port', database.get('port', ''))) - username = connection_params['username'] or database.get('restore_username', database.get('username')) - password = connection_params['password'] or database.get('restore_password', database.get('password')) + username = connection_params['username'] or database.get( + 'restore_username', database.get('username') + ) + password = connection_params['password'] or database.get( + 'restore_password', database.get('password') + ) command = ['mongorestore'] if extract_process: diff --git a/borgmatic/hooks/mysql.py b/borgmatic/hooks/mysql.py index f64f16f..22eef61 100644 --- a/borgmatic/hooks/mysql.py +++ b/borgmatic/hooks/mysql.py @@ -185,7 +185,9 @@ def make_database_dump_pattern( return dump.make_database_dump_filename(make_dump_path(location_config), name, hostname='*') -def restore_database_dump(database_config, log_prefix, location_config, dry_run, extract_process, connection_params): +def restore_database_dump( + database_config, log_prefix, location_config, dry_run, extract_process, connection_params +): ''' Restore the given MySQL/MariaDB database from an extract stream. The database is supplied as a one-element sequence containing a dict describing the database, as per the configuration schema. @@ -200,10 +202,16 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, database = database_config[0] - hostname = connection_params['hostname'] or database.get('restore_hostname', database.get('hostname')) + hostname = connection_params['hostname'] or database.get( + 'restore_hostname', database.get('hostname') + ) port = str(connection_params['port'] or database.get('restore_port', database.get('port', ''))) - username = connection_params['username'] or database.get('restore_username', database.get('username')) - password = connection_params['password'] or database.get('restore_password', database.get('password')) + username = connection_params['username'] or database.get( + 'restore_username', database.get('username') + ) + password = connection_params['password'] or database.get( + 'restore_password', database.get('password') + ) restore_command = ( ('mysql', '--batch') diff --git a/borgmatic/hooks/postgresql.py b/borgmatic/hooks/postgresql.py index 08b0019..ecb5f3c 100644 --- a/borgmatic/hooks/postgresql.py +++ b/borgmatic/hooks/postgresql.py @@ -22,6 +22,7 @@ def make_dump_path(location_config): # pragma: no cover location_config.get('borgmatic_source_directory'), 'postgresql_databases' ) + def make_extra_environment(database, restore_connection_params=None): ''' Make the extra_environment dict from the given database configuration. @@ -31,12 +32,14 @@ def make_extra_environment(database, restore_connection_params=None): try: if restore_connection_params: - extra['PGPASSWORD'] = restore_connection_params.get('password') or database.get('restore_password', database['password']) + extra['PGPASSWORD'] = restore_connection_params.get('password') or database.get( + 'restore_password', database['password'] + ) else: extra['PGPASSWORD'] = database['password'] except (AttributeError, KeyError): pass - + extra['PGSSLMODE'] = database.get('ssl_mode', 'disable') if 'ssl_cert' in database: extra['PGSSLCERT'] = database['ssl_cert'] @@ -200,7 +203,9 @@ def make_database_dump_pattern( return dump.make_database_dump_filename(make_dump_path(location_config), name, hostname='*') -def restore_database_dump(database_config, log_prefix, location_config, dry_run, extract_process, connection_params): +def restore_database_dump( + database_config, log_prefix, location_config, dry_run, extract_process, connection_params +): ''' Restore the given PostgreSQL database from an extract stream. The database is supplied as a one-element sequence containing a dict describing the database, as per the configuration schema. @@ -221,9 +226,13 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, database = database_config[0] - hostname = connection_params['hostname'] or database.get('restore_hostname', database.get('hostname')) + hostname = connection_params['hostname'] or database.get( + 'restore_hostname', database.get('hostname') + ) port = str(connection_params['port'] or database.get('restore_port', database.get('port', ''))) - username = connection_params['username'] or database.get('restore_username', database.get('username')) + username = connection_params['username'] or database.get( + 'restore_username', database.get('username') + ) all_databases = bool(database['name'] == 'all') dump_filename = dump.make_database_dump_filename( @@ -260,7 +269,9 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, ) ) - extra_environment = make_extra_environment(database, restore_connection_params=connection_params) + extra_environment = make_extra_environment( + database, restore_connection_params=connection_params + ) logger.debug(f"{log_prefix}: Restoring PostgreSQL database {database['name']}{dry_run_label}") if dry_run: diff --git a/borgmatic/hooks/sqlite.py b/borgmatic/hooks/sqlite.py index 4e4e0be..21b1455 100644 --- a/borgmatic/hooks/sqlite.py +++ b/borgmatic/hooks/sqlite.py @@ -85,7 +85,9 @@ def make_database_dump_pattern( return dump.make_database_dump_filename(make_dump_path(location_config), name) -def restore_database_dump(database_config, log_prefix, location_config, dry_run, extract_process, connection_params): +def restore_database_dump( + database_config, log_prefix, location_config, dry_run, extract_process, connection_params +): ''' Restore the given SQLite3 database from an extract stream. The database is supplied as a one-element sequence containing a dict describing the database, as per the configuration schema. @@ -98,7 +100,9 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run, if len(database_config) != 1: raise ValueError('The database configuration value is invalid') - database_path = connection_params['restore_path'] or database_config[0].get('restore_path', database_config[0].get('path')) + database_path = connection_params['restore_path'] or database_config[0].get( + 'restore_path', database_config[0].get('path') + ) logger.debug(f'{log_prefix}: Restoring SQLite database at {database_path}{dry_run_label}') if dry_run: diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index 4bad6f8..4e19964 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -241,6 +241,7 @@ def test_run_restore_restores_each_database(): archive_name=object, hook_name='postgresql_databases', database={'name': 'foo', 'schemas': None}, + connection_params=object, ).once() flexmock(module).should_receive('restore_single_database').with_args( repository=object, @@ -254,6 +255,7 @@ def test_run_restore_restores_each_database(): archive_name=object, hook_name='postgresql_databases', database={'name': 'bar', 'schemas': None}, + connection_params=object, ).once() flexmock(module).should_receive('ensure_databases_found') @@ -264,7 +266,15 @@ def test_run_restore_restores_each_database(): hooks=flexmock(), local_borg_version=flexmock(), restore_arguments=flexmock( - repository='repo', archive='archive', databases=flexmock(), schemas=None + repository='repo', + archive='archive', + databases=flexmock(), + schemas=None, + hostname=None, + port=None, + username=None, + password=None, + restore_path=None, ), global_arguments=flexmock(dry_run=False), local_path=flexmock(), @@ -337,6 +347,7 @@ def test_run_restore_restores_database_configured_with_all_name(): archive_name=object, hook_name='postgresql_databases', database={'name': 'foo', 'schemas': None}, + connection_params=object, ).once() flexmock(module).should_receive('restore_single_database').with_args( repository=object, @@ -350,6 +361,7 @@ def test_run_restore_restores_database_configured_with_all_name(): archive_name=object, hook_name='postgresql_databases', database={'name': 'bar', 'schemas': None}, + connection_params=object, ).once() flexmock(module).should_receive('ensure_databases_found') @@ -360,7 +372,15 @@ def test_run_restore_restores_database_configured_with_all_name(): hooks=flexmock(), local_borg_version=flexmock(), restore_arguments=flexmock( - repository='repo', archive='archive', databases=flexmock(), schemas=None + repository='repo', + archive='archive', + databases=flexmock(), + schemas=None, + hostname=None, + port=None, + username=None, + password=None, + restore_path=None, ), global_arguments=flexmock(dry_run=False), local_path=flexmock(), @@ -411,6 +431,7 @@ def test_run_restore_skips_missing_database(): archive_name=object, hook_name='postgresql_databases', database={'name': 'foo', 'schemas': None}, + connection_params=object, ).once() flexmock(module).should_receive('restore_single_database').with_args( repository=object, @@ -424,6 +445,7 @@ def test_run_restore_skips_missing_database(): archive_name=object, hook_name='postgresql_databases', database={'name': 'bar', 'schemas': None}, + connection_params=object, ).never() flexmock(module).should_receive('ensure_databases_found') @@ -434,7 +456,15 @@ def test_run_restore_skips_missing_database(): hooks=flexmock(), local_borg_version=flexmock(), restore_arguments=flexmock( - repository='repo', archive='archive', databases=flexmock(), schemas=None + repository='repo', + archive='archive', + databases=flexmock(), + schemas=None, + hostname=None, + port=None, + username=None, + password=None, + restore_path=None, ), global_arguments=flexmock(dry_run=False), local_path=flexmock(), @@ -479,6 +509,7 @@ def test_run_restore_restores_databases_from_different_hooks(): archive_name=object, hook_name='postgresql_databases', database={'name': 'foo', 'schemas': None}, + connection_params=object, ).once() flexmock(module).should_receive('restore_single_database').with_args( repository=object, @@ -492,6 +523,7 @@ def test_run_restore_restores_databases_from_different_hooks(): archive_name=object, hook_name='mysql_databases', database={'name': 'bar', 'schemas': None}, + connection_params=object, ).once() flexmock(module).should_receive('ensure_databases_found') @@ -502,7 +534,15 @@ def test_run_restore_restores_databases_from_different_hooks(): hooks=flexmock(), local_borg_version=flexmock(), restore_arguments=flexmock( - repository='repo', archive='archive', databases=flexmock(), schemas=None + repository='repo', + archive='archive', + databases=flexmock(), + schemas=None, + hostname=None, + port=None, + username=None, + password=None, + restore_path=None, ), global_arguments=flexmock(dry_run=False), local_path=flexmock(), diff --git a/tests/unit/hooks/test_mongodb.py b/tests/unit/hooks/test_mongodb.py index f038a88..2c09fef 100644 --- a/tests/unit/hooks/test_mongodb.py +++ b/tests/unit/hooks/test_mongodb.py @@ -171,7 +171,17 @@ def test_restore_database_dump_runs_mongorestore(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -185,7 +195,17 @@ def test_restore_database_dump_errors_on_multiple_database_config(): with pytest.raises(ValueError): module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=flexmock() + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=flexmock(), + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -215,7 +235,17 @@ def test_restore_database_dump_runs_mongorestore_with_hostname_and_port(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -253,7 +283,17 @@ def test_restore_database_dump_runs_mongorestore_with_username_and_password(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -271,7 +311,17 @@ def test_restore_database_dump_runs_mongorestore_with_options(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -299,7 +349,17 @@ def test_restore_databases_dump_runs_mongorestore_with_schemas(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -317,7 +377,17 @@ def test_restore_database_dump_runs_psql_for_all_database_dump(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -329,7 +399,17 @@ def test_restore_database_dump_with_dry_run_skips_restore(): flexmock(module).should_receive('execute_command_with_processes').never() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=True, extract_process=flexmock() + database_config, + 'test.yaml', + {}, + dry_run=True, + extract_process=flexmock(), + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -346,5 +426,15 @@ def test_restore_database_dump_without_extract_process_restores_from_disk(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=None + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=None, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) diff --git a/tests/unit/hooks/test_mysql.py b/tests/unit/hooks/test_mysql.py index da5da16..9b65b12 100644 --- a/tests/unit/hooks/test_mysql.py +++ b/tests/unit/hooks/test_mysql.py @@ -392,7 +392,17 @@ def test_restore_database_dump_runs_mysql_to_restore(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -404,7 +414,17 @@ def test_restore_database_dump_errors_on_multiple_database_config(): with pytest.raises(ValueError): module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=flexmock() + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=flexmock(), + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -421,7 +441,17 @@ def test_restore_database_dump_runs_mysql_with_options(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -447,7 +477,17 @@ def test_restore_database_dump_runs_mysql_with_hostname_and_port(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -464,7 +504,17 @@ def test_restore_database_dump_runs_mysql_with_username_and_password(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -474,5 +524,15 @@ def test_restore_database_dump_with_dry_run_skips_restore(): flexmock(module).should_receive('execute_command_with_processes').never() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=True, extract_process=flexmock() + database_config, + 'test.yaml', + {}, + dry_run=True, + extract_process=flexmock(), + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) diff --git a/tests/unit/hooks/test_postgresql.py b/tests/unit/hooks/test_postgresql.py index b3a55fa..3baf4dc 100644 --- a/tests/unit/hooks/test_postgresql.py +++ b/tests/unit/hooks/test_postgresql.py @@ -479,7 +479,17 @@ def test_restore_database_dump_runs_pg_restore(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -494,7 +504,17 @@ def test_restore_database_dump_errors_on_multiple_database_config(): with pytest.raises(ValueError): module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=flexmock() + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=flexmock(), + connection_params={ + 'restore_hostname': None, + 'restore_port': None, + 'restore_username': None, + 'restore_password': None, + }, ) @@ -545,7 +565,17 @@ def test_restore_database_dump_runs_pg_restore_with_hostname_and_port(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -594,7 +624,17 @@ def test_restore_database_dump_runs_pg_restore_with_username_and_password(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -644,7 +684,17 @@ def test_restore_database_dump_runs_pg_restore_with_options(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -672,7 +722,17 @@ def test_restore_database_dump_runs_psql_for_all_database_dump(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -705,7 +765,17 @@ def test_restore_database_dump_runs_psql_for_plain_database_dump(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -759,7 +829,17 @@ def test_restore_database_dump_runs_non_default_pg_restore_and_psql(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -772,7 +852,17 @@ def test_restore_database_dump_with_dry_run_skips_restore(): flexmock(module).should_receive('execute_command_with_processes').never() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=True, extract_process=flexmock() + database_config, + 'test.yaml', + {}, + dry_run=True, + extract_process=flexmock(), + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -813,7 +903,17 @@ def test_restore_database_dump_without_extract_process_restores_from_disk(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=None + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=None, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) @@ -858,5 +958,15 @@ def test_restore_database_dump_with_schemas_restores_schemas(): ).once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=None + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=None, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, ) diff --git a/tests/unit/hooks/test_sqlite.py b/tests/unit/hooks/test_sqlite.py index a660d81..2edd4d1 100644 --- a/tests/unit/hooks/test_sqlite.py +++ b/tests/unit/hooks/test_sqlite.py @@ -99,7 +99,12 @@ def test_restore_database_dump_restores_database(): flexmock(module.os).should_receive('remove').once() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={'restore_path': None}, ) @@ -111,7 +116,12 @@ def test_restore_database_dump_does_not_restore_database_if_dry_run(): flexmock(module.os).should_receive('remove').never() module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=True, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=True, + extract_process=extract_process, + connection_params={'restore_path': None}, ) @@ -121,5 +131,10 @@ def test_restore_database_dump_raises_error_if_database_config_is_invalid(): with pytest.raises(ValueError): module.restore_database_dump( - database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={'restore_path': None}, ) From 6c876085488b8ab9aef7cc9bab3da6534df790c1 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Sat, 17 Jun 2023 00:47:15 +0530 Subject: [PATCH 12/18] add tests for password logic --- tests/unit/hooks/test_postgresql.py | 108 ++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/tests/unit/hooks/test_postgresql.py b/tests/unit/hooks/test_postgresql.py index 3baf4dc..30ed7e3 100644 --- a/tests/unit/hooks/test_postgresql.py +++ b/tests/unit/hooks/test_postgresql.py @@ -638,6 +638,114 @@ def test_restore_database_dump_runs_pg_restore_with_username_and_password(): ) +def test_restore_database_dump_with_cli_password_runs_pg_restore_with_password(): + database_config = [{'name': 'foo', 'username': 'postgres', 'schemas': None}] + extract_process = flexmock(stdout=flexmock()) + + flexmock(module).should_receive('make_dump_path') + flexmock(module.dump).should_receive('make_database_dump_filename') + flexmock(module).should_receive('execute_command_with_processes').with_args( + ( + 'pg_restore', + '--no-password', + '--if-exists', + '--exit-on-error', + '--clean', + '--dbname', + 'foo', + '--username', + 'postgres', + ), + processes=[extract_process], + output_log_level=logging.DEBUG, + input_file=extract_process.stdout, + extra_environment={'PGPASSWORD': 'trustsome1', 'PGSSLMODE': 'disable'}, + ).once() + flexmock(module).should_receive('execute_command').with_args( + ( + 'psql', + '--no-password', + '--no-psqlrc', + '--quiet', + '--username', + 'postgres', + '--dbname', + 'foo', + '--command', + 'ANALYZE', + ), + extra_environment={'PGPASSWORD': 'trustsome1', 'PGSSLMODE': 'disable'}, + ).once() + + module.restore_database_dump( + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': 'trustsome1', + }, + ) + + +def test_restore_database_dump_with_no_passwords_runs_pg_restore_without_password(): + database_config = [{'name': 'foo', 'username': 'postgres', 'schemas': None}] + extract_process = flexmock(stdout=flexmock()) + + flexmock(module).should_receive('make_dump_path') + flexmock(module.dump).should_receive('make_database_dump_filename') + flexmock(module).should_receive('execute_command_with_processes').with_args( + ( + 'pg_restore', + '--no-password', + '--if-exists', + '--exit-on-error', + '--clean', + '--dbname', + 'foo', + '--username', + 'postgres', + ), + processes=[extract_process], + output_log_level=logging.DEBUG, + input_file=extract_process.stdout, + extra_environment={'PGSSLMODE': 'disable'}, + ).once() + flexmock(module).should_receive('execute_command').with_args( + ( + 'psql', + '--no-password', + '--no-psqlrc', + '--quiet', + '--username', + 'postgres', + '--dbname', + 'foo', + '--command', + 'ANALYZE', + ), + extra_environment={'PGSSLMODE': 'disable'}, + ).once() + + module.restore_database_dump( + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, + ) + + def test_restore_database_dump_runs_pg_restore_with_options(): database_config = [ { From e53dd3da87bbd743061ad8e46dbf0a8a627f1918 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Sat, 17 Jun 2023 22:58:59 +0530 Subject: [PATCH 13/18] fix witten reported mysql error --- borgmatic/hooks/mysql.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/borgmatic/hooks/mysql.py b/borgmatic/hooks/mysql.py index 22eef61..aee13d8 100644 --- a/borgmatic/hooks/mysql.py +++ b/borgmatic/hooks/mysql.py @@ -216,10 +216,10 @@ def restore_database_dump( restore_command = ( ('mysql', '--batch') + (tuple(database['restore_options'].split(' ')) if 'restore_options' in database else ()) - + (('--host', database['hostname']) if hostname else ()) - + (('--port', str(database['port'])) if port else ()) + + (('--host', hostname) if hostname else ()) + + (('--port', str(port)) if port else ()) + (('--protocol', 'tcp') if hostname or port else ()) - + (('--user', database['username']) if username else ()) + + (('--user', username) if username else ()) ) extra_environment = {'MYSQL_PWD': password} if password else None From 9016dcc41816fa5acd74f33c931aca330660abc1 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Sun, 18 Jun 2023 05:47:35 +0530 Subject: [PATCH 14/18] all e2e tests --- borgmatic/config/schema.yaml | 1 + tests/end-to-end/docker-compose.yaml | 25 ++++ tests/end-to-end/test_database.py | 187 +++++++++++++++++++++++++++ 3 files changed, 213 insertions(+) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 7d079ff..fb8d7b1 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -818,6 +818,7 @@ properties: These statements will fail unless the initial connection to the database is made by a superuser. + example: true format: type: string enum: ['plain', 'custom', 'directory', 'tar'] diff --git a/tests/end-to-end/docker-compose.yaml b/tests/end-to-end/docker-compose.yaml index 0bbec8c..0aa19c5 100644 --- a/tests/end-to-end/docker-compose.yaml +++ b/tests/end-to-end/docker-compose.yaml @@ -5,16 +5,38 @@ services: environment: POSTGRES_PASSWORD: test POSTGRES_DB: test + postgresql2: + image: docker.io/postgres:13.1-alpine + environment: + POSTGRES_PASSWORD: test2 + POSTGRES_DB: test + POSTGRES_USER: postgres2 + ports: + - "5433:5432" mysql: image: docker.io/mariadb:10.5 environment: MYSQL_ROOT_PASSWORD: test MYSQL_DATABASE: test + mysql2: + image: docker.io/mariadb:10.5 + environment: + MYSQL_ROOT_PASSWORD: test2 + MYSQL_DATABASE: test + ports: + - "3307:3306" mongodb: image: docker.io/mongo:5.0.5 environment: MONGO_INITDB_ROOT_USERNAME: root MONGO_INITDB_ROOT_PASSWORD: test + mongodb2: + image: docker.io/mongo:5.0.5 + environment: + MONGO_INITDB_ROOT_USERNAME: root2 + MONGO_INITDB_ROOT_PASSWORD: test2 + ports: + - "27018:27017" tests: image: docker.io/alpine:3.13 environment: @@ -30,5 +52,8 @@ services: command: --end-to-end-only depends_on: - postgresql + - postgresql2 - mysql + - mysql2 - mongodb + - mongodb2 diff --git a/tests/end-to-end/test_database.py b/tests/end-to-end/test_database.py index 5c4e22c..b180acb 100644 --- a/tests/end-to-end/test_database.py +++ b/tests/end-to-end/test_database.py @@ -81,6 +81,107 @@ hooks: with open(config_path, 'w') as config_file: config_file.write(config) +def write_custom_restore_configuration( + source_directory, + config_path, + repository_path, + borgmatic_source_directory, + postgresql_dump_format='custom', + mongodb_dump_format='archive', +): + ''' + Write out borgmatic configuration into a file at the config path. Set the options so as to work + for testing with custom restore options. This includes a custom restore_hostname, restore_port, + restore_username, restore_password and restore_path. + ''' + config = f''' +location: + source_directories: + - {source_directory} + repositories: + - {repository_path} + borgmatic_source_directory: {borgmatic_source_directory} + +storage: + encryption_passphrase: "test" + +hooks: + postgresql_databases: + - name: test + hostname: postgresql + username: postgres + password: test + format: {postgresql_dump_format} + restore_hostname: postgresql2 + restore_port: 5432 + restore_username: postgres2 + restore_password: test2 + mysql_databases: + - name: test + hostname: mysql + username: root + password: test + restore_hostname: mysql2 + restore_port: 3306 + restore_username: root + restore_password: test2 + mongodb_databases: + - name: test + hostname: mongodb + username: root + password: test + authentication_database: admin + format: {mongodb_dump_format} + restore_hostname: mongodb2 + restore_port: 27017 + restore_username: root2 + restore_password: test2 + sqlite_databases: + - name: sqlite_test + path: /tmp/sqlite_test.db + restore_path: /tmp/sqlite_test2.db +''' + + with open(config_path, 'w') as config_file: + config_file.write(config) + + +def write_custom_restore_configuration_for_cli_arguments( + source_directory, + config_path, + repository_path, + borgmatic_source_directory, + postgresql_dump_format='custom', +): + ''' + Write out borgmatic configuration into a file at the config path. Set the options so as to work + for testing with custom restore options, but this time using CLI arguments. This includes a + custom restore_hostname, restore_port, restore_username and restore_password as we only test + these options for PostgreSQL. + ''' + config = f''' +location: + source_directories: + - {source_directory} + repositories: + - {repository_path} + borgmatic_source_directory: {borgmatic_source_directory} + +storage: + encryption_passphrase: "test" + +hooks: + postgresql_databases: + - name: test + hostname: postgresql + username: postgres + password: test + format: {postgresql_dump_format} +''' + + with open(config_path, 'w') as config_file: + config_file.write(config) + def test_database_dump_and_restore(): # Create a Borg repository. @@ -125,6 +226,92 @@ def test_database_dump_and_restore(): shutil.rmtree(temporary_directory) +def test_database_dump_and_restore_with_restore_cli_arguments(): + # Create a Borg repository. + temporary_directory = tempfile.mkdtemp() + repository_path = os.path.join(temporary_directory, 'test.borg') + borgmatic_source_directory = os.path.join(temporary_directory, '.borgmatic') + + # Write out a special file to ensure that it gets properly excluded and Borg doesn't hang on it. + os.mkfifo(os.path.join(temporary_directory, 'special_file')) + + original_working_directory = os.getcwd() + + try: + config_path = os.path.join(temporary_directory, 'test.yaml') + write_custom_restore_configuration_for_cli_arguments( + temporary_directory, config_path, repository_path, borgmatic_source_directory + ) + + subprocess.check_call( + ['borgmatic', '-v', '2', '--config', config_path, 'init', '--encryption', 'repokey'] + ) + + # Run borgmatic to generate a backup archive including a database dump. + subprocess.check_call(['borgmatic', 'create', '--config', config_path, '-v', '2']) + + # Get the created archive name. + output = subprocess.check_output( + ['borgmatic', '--config', config_path, 'list', '--json'] + ).decode(sys.stdout.encoding) + parsed_output = json.loads(output) + + assert len(parsed_output) == 1 + assert len(parsed_output[0]['archives']) == 1 + archive_name = parsed_output[0]['archives'][0]['archive'] + + # Restore the database from the archive. + subprocess.check_call( + ['borgmatic', '-v', '2', '--config', config_path, 'restore', '--archive', archive_name, '--hostname', 'postgresql2', '--port', '5432', '--username', 'postgres2', '--password', 'test2'] + ) + finally: + os.chdir(original_working_directory) + shutil.rmtree(temporary_directory) + + +def test_database_dump_and_restore_to_different_hostname_port_username_password(): + # Create a Borg repository. + temporary_directory = tempfile.mkdtemp() + repository_path = os.path.join(temporary_directory, 'test.borg') + borgmatic_source_directory = os.path.join(temporary_directory, '.borgmatic') + + # Write out a special file to ensure that it gets properly excluded and Borg doesn't hang on it. + os.mkfifo(os.path.join(temporary_directory, 'special_file')) + + original_working_directory = os.getcwd() + + try: + config_path = os.path.join(temporary_directory, 'test.yaml') + write_custom_restore_configuration( + temporary_directory, config_path, repository_path, borgmatic_source_directory + ) + + subprocess.check_call( + ['borgmatic', '-v', '2', '--config', config_path, 'init', '--encryption', 'repokey'] + ) + + # Run borgmatic to generate a backup archive including a database dump. + subprocess.check_call(['borgmatic', 'create', '--config', config_path, '-v', '2']) + + # Get the created archive name. + output = subprocess.check_output( + ['borgmatic', '--config', config_path, 'list', '--json'] + ).decode(sys.stdout.encoding) + parsed_output = json.loads(output) + + assert len(parsed_output) == 1 + assert len(parsed_output[0]['archives']) == 1 + archive_name = parsed_output[0]['archives'][0]['archive'] + + # Restore the database from the archive. + subprocess.check_call( + ['borgmatic', '-v', '2', '--config', config_path, 'restore', '--archive', archive_name] + ) + finally: + os.chdir(original_working_directory) + shutil.rmtree(temporary_directory) + + def test_database_dump_and_restore_with_directory_format(): # Create a Borg repository. temporary_directory = tempfile.mkdtemp() From 384182172a29c89c393786daeca1c9d71136ee1e Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Sun, 18 Jun 2023 06:29:11 +0530 Subject: [PATCH 15/18] add unit tests for cases when cli/config restore args are used --- tests/end-to-end/test_database.py | 20 ++++- tests/unit/hooks/test_postgresql.py | 122 ++++++++++++++++++++++++++-- 2 files changed, 134 insertions(+), 8 deletions(-) diff --git a/tests/end-to-end/test_database.py b/tests/end-to-end/test_database.py index b180acb..c424a7e 100644 --- a/tests/end-to-end/test_database.py +++ b/tests/end-to-end/test_database.py @@ -81,6 +81,7 @@ hooks: with open(config_path, 'w') as config_file: config_file.write(config) + def write_custom_restore_configuration( source_directory, config_path, @@ -262,7 +263,24 @@ def test_database_dump_and_restore_with_restore_cli_arguments(): # Restore the database from the archive. subprocess.check_call( - ['borgmatic', '-v', '2', '--config', config_path, 'restore', '--archive', archive_name, '--hostname', 'postgresql2', '--port', '5432', '--username', 'postgres2', '--password', 'test2'] + [ + 'borgmatic', + '-v', + '2', + '--config', + config_path, + 'restore', + '--archive', + archive_name, + '--hostname', + 'postgresql2', + '--port', + '5432', + '--username', + 'postgres2', + '--password', + 'test2', + ] ) finally: os.chdir(original_working_directory) diff --git a/tests/unit/hooks/test_postgresql.py b/tests/unit/hooks/test_postgresql.py index 30ed7e3..60bbe57 100644 --- a/tests/unit/hooks/test_postgresql.py +++ b/tests/unit/hooks/test_postgresql.py @@ -638,10 +638,32 @@ def test_restore_database_dump_runs_pg_restore_with_username_and_password(): ) -def test_restore_database_dump_with_cli_password_runs_pg_restore_with_password(): - database_config = [{'name': 'foo', 'username': 'postgres', 'schemas': None}] +def test_make_extra_environment_with_cli_password_sets_correct_password(): + database = {'name': 'foo', 'restore_password': 'trustsome1', 'password': 'anotherpassword'} + + extra = module.make_extra_environment( + database, restore_connection_params={'password': 'clipassword'} + ) + + assert extra['PGPASSWORD'] == 'clipassword' + + +def test_restore_database_dump_with_connection_params_uses_connection_params_for_restore(): + database_config = [ + { + 'name': 'foo', + 'hostname': 'database.example.org', + 'port': 5433, + 'username': 'postgres', + 'password': 'trustsome1', + 'schemas': None, + } + ] extract_process = flexmock(stdout=flexmock()) + flexmock(module).should_receive('make_extra_environment').and_return( + {'PGPASSWORD': 'clipassword', 'PGSSLMODE': 'disable'} + ) flexmock(module).should_receive('make_dump_path') flexmock(module.dump).should_receive('make_database_dump_filename') flexmock(module).should_receive('execute_command_with_processes').with_args( @@ -653,13 +675,17 @@ def test_restore_database_dump_with_cli_password_runs_pg_restore_with_password() '--clean', '--dbname', 'foo', + '--host', + 'clihost', + '--port', + 'cliport', '--username', - 'postgres', + 'cliusername', ), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - extra_environment={'PGPASSWORD': 'trustsome1', 'PGSSLMODE': 'disable'}, + extra_environment={'PGPASSWORD': 'clipassword', 'PGSSLMODE': 'disable'}, ).once() flexmock(module).should_receive('execute_command').with_args( ( @@ -667,14 +693,96 @@ def test_restore_database_dump_with_cli_password_runs_pg_restore_with_password() '--no-password', '--no-psqlrc', '--quiet', + '--host', + 'clihost', + '--port', + 'cliport', '--username', - 'postgres', + 'cliusername', '--dbname', 'foo', '--command', 'ANALYZE', ), - extra_environment={'PGPASSWORD': 'trustsome1', 'PGSSLMODE': 'disable'}, + extra_environment={'PGPASSWORD': 'clipassword', 'PGSSLMODE': 'disable'}, + ).once() + + module.restore_database_dump( + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': 'clihost', + 'port': 'cliport', + 'username': 'cliusername', + 'password': 'clipassword', + }, + ) + + +def test_restore_database_dump_without_connection_params_uses_restore_params_in_config_for_restore(): + database_config = [ + { + 'name': 'foo', + 'hostname': 'database.example.org', + 'port': 5433, + 'username': 'postgres', + 'password': 'trustsome1', + 'schemas': None, + 'restore_hostname': 'restorehost', + 'restore_port': 'restoreport', + 'restore_username': 'restoreusername', + 'restore_password': 'restorepassword', + } + ] + extract_process = flexmock(stdout=flexmock()) + + flexmock(module).should_receive('make_extra_environment').and_return( + {'PGPASSWORD': 'restorepassword', 'PGSSLMODE': 'disable'} + ) + flexmock(module).should_receive('make_dump_path') + flexmock(module.dump).should_receive('make_database_dump_filename') + flexmock(module).should_receive('execute_command_with_processes').with_args( + ( + 'pg_restore', + '--no-password', + '--if-exists', + '--exit-on-error', + '--clean', + '--dbname', + 'foo', + '--host', + 'restorehost', + '--port', + 'restoreport', + '--username', + 'restoreusername', + ), + processes=[extract_process], + output_log_level=logging.DEBUG, + input_file=extract_process.stdout, + extra_environment={'PGPASSWORD': 'restorepassword', 'PGSSLMODE': 'disable'}, + ).once() + flexmock(module).should_receive('execute_command').with_args( + ( + 'psql', + '--no-password', + '--no-psqlrc', + '--quiet', + '--host', + 'restorehost', + '--port', + 'restoreport', + '--username', + 'restoreusername', + '--dbname', + 'foo', + '--command', + 'ANALYZE', + ), + extra_environment={'PGPASSWORD': 'restorepassword', 'PGSSLMODE': 'disable'}, ).once() module.restore_database_dump( @@ -687,7 +795,7 @@ def test_restore_database_dump_with_cli_password_runs_pg_restore_with_password() 'hostname': None, 'port': None, 'username': None, - 'password': 'trustsome1', + 'password': None, }, ) From e2d82e9bba1fb053eee717c478df4aaea8629cc9 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Mon, 19 Jun 2023 01:10:01 +0530 Subject: [PATCH 16/18] actually test port restores --- tests/end-to-end/docker-compose.yaml | 3 ++ tests/end-to-end/test_database.py | 20 ++++------- tests/unit/hooks/test_postgresql.py | 54 ---------------------------- 3 files changed, 10 insertions(+), 67 deletions(-) diff --git a/tests/end-to-end/docker-compose.yaml b/tests/end-to-end/docker-compose.yaml index 0aa19c5..a769c16 100644 --- a/tests/end-to-end/docker-compose.yaml +++ b/tests/end-to-end/docker-compose.yaml @@ -13,6 +13,7 @@ services: POSTGRES_USER: postgres2 ports: - "5433:5432" + command: -p 5433 mysql: image: docker.io/mariadb:10.5 environment: @@ -25,6 +26,7 @@ services: MYSQL_DATABASE: test ports: - "3307:3306" + command: --port=3307 mongodb: image: docker.io/mongo:5.0.5 environment: @@ -37,6 +39,7 @@ services: MONGO_INITDB_ROOT_PASSWORD: test2 ports: - "27018:27017" + command: --port=27018 tests: image: docker.io/alpine:3.13 environment: diff --git a/tests/end-to-end/test_database.py b/tests/end-to-end/test_database.py index c424a7e..fd565b2 100644 --- a/tests/end-to-end/test_database.py +++ b/tests/end-to-end/test_database.py @@ -114,7 +114,7 @@ hooks: password: test format: {postgresql_dump_format} restore_hostname: postgresql2 - restore_port: 5432 + restore_port: 5433 restore_username: postgres2 restore_password: test2 mysql_databases: @@ -123,7 +123,7 @@ hooks: username: root password: test restore_hostname: mysql2 - restore_port: 3306 + restore_port: 3307 restore_username: root restore_password: test2 mongodb_databases: @@ -134,7 +134,7 @@ hooks: authentication_database: admin format: {mongodb_dump_format} restore_hostname: mongodb2 - restore_port: 27017 + restore_port: 27018 restore_username: root2 restore_password: test2 sqlite_databases: @@ -147,7 +147,7 @@ hooks: config_file.write(config) -def write_custom_restore_configuration_for_cli_arguments( +def write_simple_custom_restore_configuration( source_directory, config_path, repository_path, @@ -233,14 +233,11 @@ def test_database_dump_and_restore_with_restore_cli_arguments(): repository_path = os.path.join(temporary_directory, 'test.borg') borgmatic_source_directory = os.path.join(temporary_directory, '.borgmatic') - # Write out a special file to ensure that it gets properly excluded and Borg doesn't hang on it. - os.mkfifo(os.path.join(temporary_directory, 'special_file')) - original_working_directory = os.getcwd() try: config_path = os.path.join(temporary_directory, 'test.yaml') - write_custom_restore_configuration_for_cli_arguments( + write_simple_custom_restore_configuration( temporary_directory, config_path, repository_path, borgmatic_source_directory ) @@ -275,7 +272,7 @@ def test_database_dump_and_restore_with_restore_cli_arguments(): '--hostname', 'postgresql2', '--port', - '5432', + '5433', '--username', 'postgres2', '--password', @@ -287,15 +284,12 @@ def test_database_dump_and_restore_with_restore_cli_arguments(): shutil.rmtree(temporary_directory) -def test_database_dump_and_restore_to_different_hostname_port_username_password(): +def test_database_dump_and_restore_with_restore_configuration_options(): # Create a Borg repository. temporary_directory = tempfile.mkdtemp() repository_path = os.path.join(temporary_directory, 'test.borg') borgmatic_source_directory = os.path.join(temporary_directory, '.borgmatic') - # Write out a special file to ensure that it gets properly excluded and Borg doesn't hang on it. - os.mkfifo(os.path.join(temporary_directory, 'special_file')) - original_working_directory = os.getcwd() try: diff --git a/tests/unit/hooks/test_postgresql.py b/tests/unit/hooks/test_postgresql.py index 60bbe57..5120ad0 100644 --- a/tests/unit/hooks/test_postgresql.py +++ b/tests/unit/hooks/test_postgresql.py @@ -800,60 +800,6 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_ ) -def test_restore_database_dump_with_no_passwords_runs_pg_restore_without_password(): - database_config = [{'name': 'foo', 'username': 'postgres', 'schemas': None}] - extract_process = flexmock(stdout=flexmock()) - - flexmock(module).should_receive('make_dump_path') - flexmock(module.dump).should_receive('make_database_dump_filename') - flexmock(module).should_receive('execute_command_with_processes').with_args( - ( - 'pg_restore', - '--no-password', - '--if-exists', - '--exit-on-error', - '--clean', - '--dbname', - 'foo', - '--username', - 'postgres', - ), - processes=[extract_process], - output_log_level=logging.DEBUG, - input_file=extract_process.stdout, - extra_environment={'PGSSLMODE': 'disable'}, - ).once() - flexmock(module).should_receive('execute_command').with_args( - ( - 'psql', - '--no-password', - '--no-psqlrc', - '--quiet', - '--username', - 'postgres', - '--dbname', - 'foo', - '--command', - 'ANALYZE', - ), - extra_environment={'PGSSLMODE': 'disable'}, - ).once() - - module.restore_database_dump( - database_config, - 'test.yaml', - {}, - dry_run=False, - extract_process=extract_process, - connection_params={ - 'hostname': None, - 'port': None, - 'username': None, - 'password': None, - }, - ) - - def test_restore_database_dump_runs_pg_restore_with_options(): database_config = [ { From 1a21eb03cdc4e506575a6579b36ff758b7aa069d Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Tue, 20 Jun 2023 00:52:01 +0530 Subject: [PATCH 17/18] add tests for all databases --- tests/end-to-end/docker-compose.yaml | 6 -- tests/unit/hooks/test_mongodb.py | 108 +++++++++++++++++++++++++++ tests/unit/hooks/test_mysql.py | 88 ++++++++++++++++++++++ tests/unit/hooks/test_sqlite.py | 65 +++++++++++++++- 4 files changed, 260 insertions(+), 7 deletions(-) diff --git a/tests/end-to-end/docker-compose.yaml b/tests/end-to-end/docker-compose.yaml index a769c16..bbeb29f 100644 --- a/tests/end-to-end/docker-compose.yaml +++ b/tests/end-to-end/docker-compose.yaml @@ -11,8 +11,6 @@ services: POSTGRES_PASSWORD: test2 POSTGRES_DB: test POSTGRES_USER: postgres2 - ports: - - "5433:5432" command: -p 5433 mysql: image: docker.io/mariadb:10.5 @@ -24,8 +22,6 @@ services: environment: MYSQL_ROOT_PASSWORD: test2 MYSQL_DATABASE: test - ports: - - "3307:3306" command: --port=3307 mongodb: image: docker.io/mongo:5.0.5 @@ -37,8 +33,6 @@ services: environment: MONGO_INITDB_ROOT_USERNAME: root2 MONGO_INITDB_ROOT_PASSWORD: test2 - ports: - - "27018:27017" command: --port=27018 tests: image: docker.io/alpine:3.13 diff --git a/tests/unit/hooks/test_mongodb.py b/tests/unit/hooks/test_mongodb.py index 2c09fef..14f7caf 100644 --- a/tests/unit/hooks/test_mongodb.py +++ b/tests/unit/hooks/test_mongodb.py @@ -297,6 +297,114 @@ def test_restore_database_dump_runs_mongorestore_with_username_and_password(): ) +def test_restore_database_dump_with_connection_params_uses_connection_params_for_restore(): + database_config = [ + { + 'name': 'foo', + 'username': 'mongo', + 'password': 'trustsome1', + 'authentication_database': 'admin', + 'schemas': None, + } + ] + extract_process = flexmock(stdout=flexmock()) + + flexmock(module).should_receive('make_dump_path') + flexmock(module.dump).should_receive('make_database_dump_filename') + flexmock(module).should_receive('execute_command_with_processes').with_args( + [ + 'mongorestore', + '--archive', + '--drop', + '--db', + 'foo', + '--host', + 'clihost', + '--port', + 'cliport', + '--username', + 'cliusername', + '--password', + 'clipassword', + '--authenticationDatabase', + 'admin', + ], + processes=[extract_process], + output_log_level=logging.DEBUG, + input_file=extract_process.stdout, + ).once() + + module.restore_database_dump( + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': 'clihost', + 'port': 'cliport', + 'username': 'cliusername', + 'password': 'clipassword', + }, + ) + + +def test_restore_database_dump_without_connection_params_uses_restore_params_in_config_for_restore(): + database_config = [ + { + 'name': 'foo', + 'username': 'mongo', + 'password': 'trustsome1', + 'authentication_database': 'admin', + 'schemas': None, + 'restore_hostname': 'restorehost', + 'restore_port': 'restoreport', + 'restore_username': 'restoreuser', + 'restore_password': 'restorepass', + } + ] + extract_process = flexmock(stdout=flexmock()) + + flexmock(module).should_receive('make_dump_path') + flexmock(module.dump).should_receive('make_database_dump_filename') + flexmock(module).should_receive('execute_command_with_processes').with_args( + [ + 'mongorestore', + '--archive', + '--drop', + '--db', + 'foo', + '--host', + 'restorehost', + '--port', + 'restoreport', + '--username', + 'restoreuser', + '--password', + 'restorepass', + '--authenticationDatabase', + 'admin', + ], + processes=[extract_process], + output_log_level=logging.DEBUG, + input_file=extract_process.stdout, + ).once() + + module.restore_database_dump( + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, + ) + + def test_restore_database_dump_runs_mongorestore_with_options(): database_config = [{'name': 'foo', 'restore_options': '--harder', 'schemas': None}] extract_process = flexmock(stdout=flexmock()) diff --git a/tests/unit/hooks/test_mysql.py b/tests/unit/hooks/test_mysql.py index 9b65b12..e45fa56 100644 --- a/tests/unit/hooks/test_mysql.py +++ b/tests/unit/hooks/test_mysql.py @@ -518,6 +518,94 @@ def test_restore_database_dump_runs_mysql_with_username_and_password(): ) +def test_restore_database_dump_with_connection_params_uses_connection_params_for_restore(): + database_config = [{'name': 'foo', 'username': 'root', 'password': 'trustsome1'}] + extract_process = flexmock(stdout=flexmock()) + + flexmock(module).should_receive('execute_command_with_processes').with_args( + ( + 'mysql', + '--batch', + '--host', + 'clihost', + '--port', + 'cliport', + '--protocol', + 'tcp', + '--user', + 'cliusername', + ), + processes=[extract_process], + output_log_level=logging.DEBUG, + input_file=extract_process.stdout, + extra_environment={'MYSQL_PWD': 'clipassword'}, + ).once() + + module.restore_database_dump( + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': 'clihost', + 'port': 'cliport', + 'username': 'cliusername', + 'password': 'clipassword', + }, + ) + + +def test_restore_database_dump_without_connection_params_uses_restore_params_in_config_for_restore(): + database_config = [ + { + 'name': 'foo', + 'username': 'root', + 'password': 'trustsome1', + 'hostname': 'dbhost', + 'port': 'dbport', + 'restore_username': 'restoreuser', + 'restore_password': 'restorepass', + 'restore_hostname': 'restorehost', + 'restore_port': 'restoreport', + } + ] + extract_process = flexmock(stdout=flexmock()) + + flexmock(module).should_receive('execute_command_with_processes').with_args( + ( + 'mysql', + '--batch', + '--host', + 'restorehost', + '--port', + 'restoreport', + '--protocol', + 'tcp', + '--user', + 'restoreuser', + ), + processes=[extract_process], + output_log_level=logging.DEBUG, + input_file=extract_process.stdout, + extra_environment={'MYSQL_PWD': 'restorepass'}, + ).once() + + module.restore_database_dump( + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={ + 'hostname': None, + 'port': None, + 'username': None, + 'password': None, + }, + ) + + def test_restore_database_dump_with_dry_run_skips_restore(): database_config = [{'name': 'foo'}] diff --git a/tests/unit/hooks/test_sqlite.py b/tests/unit/hooks/test_sqlite.py index 2edd4d1..30700e6 100644 --- a/tests/unit/hooks/test_sqlite.py +++ b/tests/unit/hooks/test_sqlite.py @@ -1,3 +1,4 @@ +import logging import pytest from flexmock import flexmock @@ -94,7 +95,69 @@ def test_restore_database_dump_restores_database(): database_config = [{'path': '/path/to/database', 'name': 'database'}] extract_process = flexmock(stdout=flexmock()) - flexmock(module).should_receive('execute_command_with_processes').once() + flexmock(module).should_receive('execute_command_with_processes').with_args( + ( + 'sqlite3', + '/path/to/database', + ), + processes=[extract_process], + output_log_level=logging.DEBUG, + input_file=extract_process.stdout, + ).once() + + flexmock(module.os).should_receive('remove').once() + + module.restore_database_dump( + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={'restore_path': None}, + ) + + +def test_restore_database_dump_with_connection_params_uses_connection_params_for_restore(): + database_config = [{'path': '/path/to/database', 'name': 'database'}] + extract_process = flexmock(stdout=flexmock()) + + flexmock(module).should_receive('execute_command_with_processes').with_args( + ( + 'sqlite3', + 'cli/path/to/database', + ), + processes=[extract_process], + output_log_level=logging.DEBUG, + input_file=extract_process.stdout, + ).once() + + flexmock(module.os).should_receive('remove').once() + + module.restore_database_dump( + database_config, + 'test.yaml', + {}, + dry_run=False, + extract_process=extract_process, + connection_params={'restore_path': 'cli/path/to/database'}, + ) + + +def test_restore_database_dump_without_connection_params_uses_restore_params_in_config_for_restore(): + database_config = [ + {'path': '/path/to/database', 'name': 'database', 'restore_path': 'config/path/to/database'} + ] + extract_process = flexmock(stdout=flexmock()) + + flexmock(module).should_receive('execute_command_with_processes').with_args( + ( + 'sqlite3', + 'config/path/to/database', + ), + processes=[extract_process], + output_log_level=logging.DEBUG, + input_file=extract_process.stdout, + ).once() flexmock(module.os).should_receive('remove').once() From 87c6e5b349710535b01e607b6135e22509482374 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Wed, 21 Jun 2023 00:03:07 +0530 Subject: [PATCH 18/18] make sure restore params in config aren't used when cli args are supplied --- tests/unit/hooks/test_mongodb.py | 4 ++++ tests/unit/hooks/test_mysql.py | 12 +++++++++++- tests/unit/hooks/test_postgresql.py | 4 ++++ tests/unit/hooks/test_sqlite.py | 4 +++- 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/tests/unit/hooks/test_mongodb.py b/tests/unit/hooks/test_mongodb.py index 14f7caf..5ac8ce9 100644 --- a/tests/unit/hooks/test_mongodb.py +++ b/tests/unit/hooks/test_mongodb.py @@ -304,6 +304,10 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for 'username': 'mongo', 'password': 'trustsome1', 'authentication_database': 'admin', + 'restore_hostname': 'restorehost', + 'restore_port': 'restoreport', + 'restore_username': 'restoreusername', + 'restore_password': 'restorepassword', 'schemas': None, } ] diff --git a/tests/unit/hooks/test_mysql.py b/tests/unit/hooks/test_mysql.py index e45fa56..cdcddf5 100644 --- a/tests/unit/hooks/test_mysql.py +++ b/tests/unit/hooks/test_mysql.py @@ -519,7 +519,17 @@ def test_restore_database_dump_runs_mysql_with_username_and_password(): def test_restore_database_dump_with_connection_params_uses_connection_params_for_restore(): - database_config = [{'name': 'foo', 'username': 'root', 'password': 'trustsome1'}] + database_config = [ + { + 'name': 'foo', + 'username': 'root', + 'password': 'trustsome1', + 'restore_hostname': 'restorehost', + 'restore_port': 'restoreport', + 'restore_username': 'restoreusername', + 'restore_password': 'restorepassword', + } + ] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('execute_command_with_processes').with_args( diff --git a/tests/unit/hooks/test_postgresql.py b/tests/unit/hooks/test_postgresql.py index 5120ad0..e48258e 100644 --- a/tests/unit/hooks/test_postgresql.py +++ b/tests/unit/hooks/test_postgresql.py @@ -656,6 +656,10 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for 'port': 5433, 'username': 'postgres', 'password': 'trustsome1', + 'restore_hostname': 'restorehost', + 'restore_port': 'restoreport', + 'restore_username': 'restoreusername', + 'restore_password': 'restorepassword', 'schemas': None, } ] diff --git a/tests/unit/hooks/test_sqlite.py b/tests/unit/hooks/test_sqlite.py index 30700e6..5820713 100644 --- a/tests/unit/hooks/test_sqlite.py +++ b/tests/unit/hooks/test_sqlite.py @@ -118,7 +118,9 @@ def test_restore_database_dump_restores_database(): def test_restore_database_dump_with_connection_params_uses_connection_params_for_restore(): - database_config = [{'path': '/path/to/database', 'name': 'database'}] + database_config = [ + {'path': '/path/to/database', 'name': 'database', 'restore_path': 'config/path/to/database'} + ] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('execute_command_with_processes').with_args(