From 0bd418836e01ac85ff5b60e4321e2d8881da20c0 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 27 Feb 2025 10:15:45 -0800 Subject: [PATCH 1/9] Send MariaDB passwords via anonymous pipe instead of environment variable (#1009) --- borgmatic/borg/environment.py | 2 +- borgmatic/execute.py | 35 +++++---- borgmatic/hooks/data_source/mariadb.py | 103 ++++++++++++++++--------- 3 files changed, 87 insertions(+), 53 deletions(-) diff --git a/borgmatic/borg/environment.py b/borgmatic/borg/environment.py index 19c8ad6e..344478c2 100644 --- a/borgmatic/borg/environment.py +++ b/borgmatic/borg/environment.py @@ -74,7 +74,7 @@ def make_environment(config): os.write(write_file_descriptor, passphrase.encode('utf-8')) os.close(write_file_descriptor) - # This, plus subprocess.Popen(..., close_fds=False) in execute.py, is necessary for the Borg + # This plus subprocess.Popen(..., close_fds=False) in execute.py is necessary for the Borg # child process to inherit the file descriptor. os.set_inheritable(read_file_descriptor, True) environment['BORG_PASSPHRASE_FD'] = str(read_file_descriptor) diff --git a/borgmatic/execute.py b/borgmatic/execute.py index df58c833..20eedb58 100644 --- a/borgmatic/execute.py +++ b/borgmatic/execute.py @@ -266,8 +266,8 @@ def log_command(full_command, input_file=None, output_file=None, environment=Non width=MAX_LOGGED_COMMAND_LENGTH, placeholder=' ...', ) - + (f" < {getattr(input_file, 'name', '')}" if input_file else '') - + (f" > {getattr(output_file, 'name', '')}" if output_file else '') + + (f" < {getattr(input_file, 'name', input_file)}" if input_file else '') + + (f" > {getattr(output_file, 'name', output_file)}" if output_file else '') ) @@ -315,8 +315,8 @@ def execute_command( shell=shell, env=environment, cwd=working_directory, - # Necessary for the passcommand credential hook to work. - close_fds=not bool((environment or {}).get('BORG_PASSPHRASE_FD')), + # Necessary for passing credentials via anonymous pipe. + close_fds=False, ) if not run_to_completion: return process @@ -333,6 +333,7 @@ def execute_command( def execute_command_and_capture_output( full_command, + input_file=None, capture_stderr=False, shell=False, environment=None, @@ -342,28 +343,30 @@ def execute_command_and_capture_output( ): ''' Execute the given command (a sequence of command/argument strings), capturing and returning its - output (stdout). If capture stderr is True, then capture and return stderr in addition to - stdout. If shell is True, execute the command within a shell. If an environment variables dict - is given, then pass it into the command. If a working directory is given, use that as the - present working directory when running the command. If a Borg local path is given, and the - command matches it (regardless of arguments), treat exit code 1 as a warning instead of an - error. But if Borg exit codes are given as a sequence of exit code configuration dicts, then use - that configuration to decide what's an error and what's a warning. + output (stdout). If an input file descriptor is given, then pipe it to the command's stdin. If + capture stderr is True, then capture and return stderr in addition to stdout. If shell is True, + execute the command within a shell. If an environment variables dict is given, then pass it into + the command. If a working directory is given, use that as the present working directory when + running the command. If a Borg local path is given, and the command matches it (regardless of + arguments), treat exit code 1 as a warning instead of an error. But if Borg exit codes are given + as a sequence of exit code configuration dicts, then use that configuration to decide what's an + error and what's a warning. Raise subprocesses.CalledProcessError if an error occurs while running the command. ''' - log_command(full_command, environment=environment) + log_command(full_command, input_file, environment=environment) command = ' '.join(full_command) if shell else full_command try: output = subprocess.check_output( command, + stdin=input_file, stderr=subprocess.STDOUT if capture_stderr else None, shell=shell, env=environment, cwd=working_directory, - # Necessary for the passcommand credential hook to work. - close_fds=not bool((environment or {}).get('BORG_PASSPHRASE_FD')), + # Necessary for passing credentials via anonymous pipe. + close_fds=False, ) except subprocess.CalledProcessError as error: if ( @@ -422,8 +425,8 @@ def execute_command_with_processes( shell=shell, env=environment, cwd=working_directory, - # Necessary for the passcommand credential hook to work. - close_fds=not bool((environment or {}).get('BORG_PASSPHRASE_FD')), + # Necessary for passing credentials via anonymous pipe. + close_fds=False, ) except (subprocess.CalledProcessError, OSError): # Something has gone wrong. So vent each process' output buffer to prevent it from hanging. diff --git a/borgmatic/hooks/data_source/mariadb.py b/borgmatic/hooks/data_source/mariadb.py index b992a944..b6be8b30 100644 --- a/borgmatic/hooks/data_source/mariadb.py +++ b/borgmatic/hooks/data_source/mariadb.py @@ -26,11 +26,56 @@ def make_dump_path(base_directory): # pragma: no cover SYSTEM_DATABASE_NAMES = ('information_schema', 'mysql', 'performance_schema', 'sys') -def database_names_to_dump(database, config, environment, dry_run): +def make_defaults_file_pipe(username=None, password=None): ''' - Given a requested database config and a configuration dict, return the corresponding sequence of - database names to dump. In the case of "all", query for the names of databases on the configured - host and return them, excluding any system databases that will cause problems during restore. + Given a database username and/or password, write it to an anonymous pipe and return its file + descriptor for passing to an executed command. The idea is that this is a more secure way to + transmit credentials to a database client than using an environment variable. + + If no username or password are given, then return None. + + Do not and use this value for multiple different command invocations. That will not work because + each pipe is "used up" once read. + ''' + values = '\n'.join( + ( + (f'user={username}' if username is not None else ''), + (f'password={password}' if password is not None else ''), + ) + ).strip() + + if not values: + return None + + fields_message = ' and '.join( + field_name for field_name in + ( + (f'username ({username})' if username is not None else None), + ('password' if password is not None else None), + ) + if field_name is not None + ) + logger.debug(f'Writing database {fields_message} to defaults extra file pipe') + + read_file_descriptor, write_file_descriptor = os.pipe() + os.write( + write_file_descriptor, f'[client]\n{values}'.encode('utf-8') + ) + os.close(write_file_descriptor) + + # This plus subprocess.Popen(..., close_fds=False) in execute.py is necessary for the database + # client child process to inherit the file descriptor. + os.set_inheritable(read_file_descriptor, True) + + return read_file_descriptor + + +def database_names_to_dump(database, config, username, password, environment, dry_run): + ''' + Given a requested database config, a configuration dict, a database username and password, an + environment dict, and whether this is a dry run, return the corresponding sequence of database + names to dump. In the case of "all", query for the names of databases on the configured host and + return them, excluding any system databases that will cause problems during restore. ''' if database['name'] != 'all': return (database['name'],) @@ -40,24 +85,20 @@ def database_names_to_dump(database, config, environment, dry_run): mariadb_show_command = tuple( shlex.quote(part) for part in shlex.split(database.get('mariadb_command') or 'mariadb') ) + defaults_file_descriptor = make_defaults_file_pipe(username, password) show_command = ( mariadb_show_command + + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + (tuple(database['list_options'].split(' ')) if 'list_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', - borgmatic.hooks.credential.parse.resolve_credential(database['username'], config), - ) - if 'username' in database - else () - ) + ('--skip-column-names', '--batch') + ('--execute', 'show schemas') ) + logger.debug('Querying for "all" MariaDB databases to dump') + show_output = execute_command_and_capture_output(show_command, environment=environment) return tuple( @@ -68,7 +109,7 @@ def database_names_to_dump(database, config, environment, dry_run): def execute_dump_command( - database, config, dump_path, database_names, environment, dry_run, dry_run_label + database, config, username, password, dump_path, database_names, environment, dry_run, dry_run_label ): ''' Kick off a dump for the given MariaDB database (provided as a configuration dict) to a named @@ -95,21 +136,15 @@ def execute_dump_command( shlex.quote(part) for part in shlex.split(database.get('mariadb_dump_command') or 'mariadb-dump') ) + defaults_file_descriptor = make_defaults_file_pipe(username, password) dump_command = ( mariadb_dump_command + + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + (tuple(database['options'].split(' ')) if 'options' in database else ()) + (('--add-drop-database',) if database.get('add_drop_database', True) 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', - borgmatic.hooks.credential.parse.resolve_credential(database['username'], config), - ) - if 'username' in database - else () - ) + ('--databases',) + database_names + ('--result-file', dump_filename) @@ -165,19 +200,10 @@ def dump_data_sources( for database in databases: dump_path = make_dump_path(borgmatic_runtime_directory) - environment = dict( - os.environ, - **( - { - 'MYSQL_PWD': borgmatic.hooks.credential.parse.resolve_credential( - database['password'], config - ) - } - if 'password' in database - else {} - ), - ) - dump_database_names = database_names_to_dump(database, config, environment, dry_run) + username = borgmatic.hooks.credential.parse.resolve_credential(database.get('username'), config) + password = borgmatic.hooks.credential.parse.resolve_credential(database.get('password'), config) + environment = dict(os.environ) + dump_database_names = database_names_to_dump(database, config, username, password, environment, dry_run) if not dump_database_names: if dry_run: @@ -193,6 +219,8 @@ def dump_data_sources( execute_dump_command( renamed_database, config, + username, + password, dump_path, (dump_name,), environment, @@ -205,6 +233,8 @@ def dump_data_sources( execute_dump_command( database, config, + username, + password, dump_path, dump_database_names, environment, @@ -296,8 +326,10 @@ def restore_data_source_dump( mariadb_restore_command = tuple( shlex.quote(part) for part in shlex.split(data_source.get('mariadb_command') or 'mariadb') ) + defaults_file_descriptor = make_defaults_file_pipe(username, password) restore_command = ( mariadb_restore_command + + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + ('--batch',) + ( tuple(data_source['restore_options'].split(' ')) @@ -307,9 +339,8 @@ def restore_data_source_dump( + (('--host', hostname) if hostname else ()) + (('--port', str(port)) if port else ()) + (('--protocol', 'tcp') if hostname or port else ()) - + (('--user', username) if username else ()) ) - environment = dict(os.environ, **({'MYSQL_PWD': password} if password else {})) + environment = dict(os.environ) logger.debug(f"Restoring MariaDB database {data_source['name']}{dry_run_label}") if dry_run: From 36d00733757b7a9123332ddf22fc4429ba9c7587 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 27 Feb 2025 10:42:47 -0800 Subject: [PATCH 2/9] Send MySQL passwords via anonymous pipe instead of environment variable (#1009). --- borgmatic/hooks/data_source/mariadb.py | 2 +- borgmatic/hooks/data_source/mysql.py | 60 +++++++++++--------------- 2 files changed, 25 insertions(+), 37 deletions(-) diff --git a/borgmatic/hooks/data_source/mariadb.py b/borgmatic/hooks/data_source/mariadb.py index b6be8b30..572b147f 100644 --- a/borgmatic/hooks/data_source/mariadb.py +++ b/borgmatic/hooks/data_source/mariadb.py @@ -34,7 +34,7 @@ def make_defaults_file_pipe(username=None, password=None): If no username or password are given, then return None. - Do not and use this value for multiple different command invocations. That will not work because + Do not use this value for multiple different command invocations. That will not work because each pipe is "used up" once read. ''' values = '\n'.join( diff --git a/borgmatic/hooks/data_source/mysql.py b/borgmatic/hooks/data_source/mysql.py index 3172edbb..23175bd6 100644 --- a/borgmatic/hooks/data_source/mysql.py +++ b/borgmatic/hooks/data_source/mysql.py @@ -6,6 +6,7 @@ import shlex import borgmatic.borg.pattern import borgmatic.config.paths import borgmatic.hooks.credential.parse +import borgmatic.hooks.data_source.mariadb from borgmatic.execute import ( execute_command, execute_command_and_capture_output, @@ -26,11 +27,12 @@ def make_dump_path(base_directory): # pragma: no cover SYSTEM_DATABASE_NAMES = ('information_schema', 'mysql', 'performance_schema', 'sys') -def database_names_to_dump(database, config, environment, dry_run): +def database_names_to_dump(database, config, username, password, environment, dry_run): ''' - Given a requested database config and a configuration dict, return the corresponding sequence of - database names to dump. In the case of "all", query for the names of databases on the configured - host and return them, excluding any system databases that will cause problems during restore. + Given a requested database config, a configuration dict, a database username and password, an + environment dict, and whether this is a dry run, return the corresponding sequence of database + names to dump. In the case of "all", query for the names of databases on the configured host and + return them, excluding any system databases that will cause problems during restore. ''' if database['name'] != 'all': return (database['name'],) @@ -40,24 +42,20 @@ def database_names_to_dump(database, config, environment, dry_run): mysql_show_command = tuple( shlex.quote(part) for part in shlex.split(database.get('mysql_command') or 'mysql') ) + defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe(username, password) show_command = ( mysql_show_command + + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + (tuple(database['list_options'].split(' ')) if 'list_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', - borgmatic.hooks.credential.parse.resolve_credential(database['username'], config), - ) - if 'username' in database - else () - ) + ('--skip-column-names', '--batch') + ('--execute', 'show schemas') ) + logger.debug('Querying for "all" MySQL databases to dump') + show_output = execute_command_and_capture_output(show_command, environment=environment) return tuple( @@ -68,7 +66,7 @@ def database_names_to_dump(database, config, environment, dry_run): def execute_dump_command( - database, config, dump_path, database_names, environment, dry_run, dry_run_label + database, config, username, password, dump_path, database_names, environment, dry_run, dry_run_label ): ''' Kick off a dump for the given MySQL/MariaDB database (provided as a configuration dict) to a @@ -94,21 +92,15 @@ def execute_dump_command( mysql_dump_command = tuple( shlex.quote(part) for part in shlex.split(database.get('mysql_dump_command') or 'mysqldump') ) + defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe(username, password) dump_command = ( mysql_dump_command + + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + (tuple(database['options'].split(' ')) if 'options' in database else ()) + (('--add-drop-database',) if database.get('add_drop_database', True) 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', - borgmatic.hooks.credential.parse.resolve_credential(database['username'], config), - ) - if 'username' in database - else () - ) + ('--databases',) + database_names + ('--result-file', dump_filename) @@ -164,19 +156,10 @@ def dump_data_sources( for database in databases: dump_path = make_dump_path(borgmatic_runtime_directory) - environment = dict( - os.environ, - **( - { - 'MYSQL_PWD': borgmatic.hooks.credential.parse.resolve_credential( - database['password'], config - ) - } - if 'password' in database - else {} - ), - ) - dump_database_names = database_names_to_dump(database, config, environment, dry_run) + username = borgmatic.hooks.credential.parse.resolve_credential(database.get('username'), config) + password = borgmatic.hooks.credential.parse.resolve_credential(database.get('password'), config) + environment = dict(os.environ) + dump_database_names = database_names_to_dump(database, config, username, password, environment, dry_run) if not dump_database_names: if dry_run: @@ -192,6 +175,8 @@ def dump_data_sources( execute_dump_command( renamed_database, config, + username, + password, dump_path, (dump_name,), environment, @@ -204,6 +189,8 @@ def dump_data_sources( execute_dump_command( database, config, + username, + password, dump_path, dump_database_names, environment, @@ -295,8 +282,10 @@ def restore_data_source_dump( mysql_restore_command = tuple( shlex.quote(part) for part in shlex.split(data_source.get('mysql_command') or 'mysql') ) + defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe(username, password) restore_command = ( mysql_restore_command + + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + ('--batch',) + ( tuple(data_source['restore_options'].split(' ')) @@ -306,9 +295,8 @@ def restore_data_source_dump( + (('--host', hostname) if hostname else ()) + (('--port', str(port)) if port else ()) + (('--protocol', 'tcp') if hostname or port else ()) - + (('--user', username) if username else ()) ) - environment = dict(os.environ, **({'MYSQL_PWD': password} if password else {})) + environment = dict(os.environ) logger.debug(f"Restoring MySQL database {data_source['name']}{dry_run_label}") if dry_run: From c41b743819b846b44e9c8dedf7cee8208428d414 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 28 Feb 2025 08:37:03 -0800 Subject: [PATCH 3/9] Get existing unit tests passing (#1009). --- borgmatic/hooks/data_source/mariadb.py | 48 ++++-- borgmatic/hooks/data_source/mysql.py | 52 +++++-- tests/unit/hooks/data_source/test_mariadb.py | 146 +++++++++++++++---- tests/unit/hooks/data_source/test_mysql.py | 146 ++++++++++++++++--- tests/unit/test_execute.py | 124 ++++------------ 5 files changed, 350 insertions(+), 166 deletions(-) diff --git a/borgmatic/hooks/data_source/mariadb.py b/borgmatic/hooks/data_source/mariadb.py index 572b147f..c57abf7a 100644 --- a/borgmatic/hooks/data_source/mariadb.py +++ b/borgmatic/hooks/data_source/mariadb.py @@ -48,8 +48,8 @@ def make_defaults_file_pipe(username=None, password=None): return None fields_message = ' and '.join( - field_name for field_name in - ( + field_name + for field_name in ( (f'username ({username})' if username is not None else None), ('password' if password is not None else None), ) @@ -58,9 +58,7 @@ def make_defaults_file_pipe(username=None, password=None): logger.debug(f'Writing database {fields_message} to defaults extra file pipe') read_file_descriptor, write_file_descriptor = os.pipe() - os.write( - write_file_descriptor, f'[client]\n{values}'.encode('utf-8') - ) + os.write(write_file_descriptor, f'[client]\n{values}'.encode('utf-8')) os.close(write_file_descriptor) # This plus subprocess.Popen(..., close_fds=False) in execute.py is necessary for the database @@ -88,7 +86,11 @@ def database_names_to_dump(database, config, username, password, environment, dr defaults_file_descriptor = make_defaults_file_pipe(username, password) show_command = ( mariadb_show_command - + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + + ( + (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) + if defaults_file_descriptor + else () + ) + (tuple(database['list_options'].split(' ')) if 'list_options' in database else ()) + (('--host', database['hostname']) if 'hostname' in database else ()) + (('--port', str(database['port'])) if 'port' in database else ()) @@ -109,7 +111,15 @@ def database_names_to_dump(database, config, username, password, environment, dr def execute_dump_command( - database, config, username, password, dump_path, database_names, environment, dry_run, dry_run_label + database, + config, + username, + password, + dump_path, + database_names, + environment, + dry_run, + dry_run_label, ): ''' Kick off a dump for the given MariaDB database (provided as a configuration dict) to a named @@ -139,7 +149,11 @@ def execute_dump_command( defaults_file_descriptor = make_defaults_file_pipe(username, password) dump_command = ( mariadb_dump_command - + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + + ( + (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) + if defaults_file_descriptor + else () + ) + (tuple(database['options'].split(' ')) if 'options' in database else ()) + (('--add-drop-database',) if database.get('add_drop_database', True) else ()) + (('--host', database['hostname']) if 'hostname' in database else ()) @@ -200,10 +214,16 @@ def dump_data_sources( for database in databases: dump_path = make_dump_path(borgmatic_runtime_directory) - username = borgmatic.hooks.credential.parse.resolve_credential(database.get('username'), config) - password = borgmatic.hooks.credential.parse.resolve_credential(database.get('password'), config) + username = borgmatic.hooks.credential.parse.resolve_credential( + database.get('username'), config + ) + password = borgmatic.hooks.credential.parse.resolve_credential( + database.get('password'), config + ) environment = dict(os.environ) - dump_database_names = database_names_to_dump(database, config, username, password, environment, dry_run) + dump_database_names = database_names_to_dump( + database, config, username, password, environment, dry_run + ) if not dump_database_names: if dry_run: @@ -329,7 +349,11 @@ def restore_data_source_dump( defaults_file_descriptor = make_defaults_file_pipe(username, password) restore_command = ( mariadb_restore_command - + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + + ( + (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) + if defaults_file_descriptor + else () + ) + ('--batch',) + ( tuple(data_source['restore_options'].split(' ')) diff --git a/borgmatic/hooks/data_source/mysql.py b/borgmatic/hooks/data_source/mysql.py index 23175bd6..a31f7a54 100644 --- a/borgmatic/hooks/data_source/mysql.py +++ b/borgmatic/hooks/data_source/mysql.py @@ -42,10 +42,16 @@ def database_names_to_dump(database, config, username, password, environment, dr mysql_show_command = tuple( shlex.quote(part) for part in shlex.split(database.get('mysql_command') or 'mysql') ) - defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe(username, password) + defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe( + username, password + ) show_command = ( mysql_show_command - + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + + ( + (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) + if defaults_file_descriptor + else () + ) + (tuple(database['list_options'].split(' ')) if 'list_options' in database else ()) + (('--host', database['hostname']) if 'hostname' in database else ()) + (('--port', str(database['port'])) if 'port' in database else ()) @@ -66,7 +72,15 @@ def database_names_to_dump(database, config, username, password, environment, dr def execute_dump_command( - database, config, username, password, dump_path, database_names, environment, dry_run, dry_run_label + database, + config, + username, + password, + dump_path, + database_names, + environment, + dry_run, + dry_run_label, ): ''' Kick off a dump for the given MySQL/MariaDB database (provided as a configuration dict) to a @@ -92,10 +106,16 @@ def execute_dump_command( mysql_dump_command = tuple( shlex.quote(part) for part in shlex.split(database.get('mysql_dump_command') or 'mysqldump') ) - defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe(username, password) + defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe( + username, password + ) dump_command = ( mysql_dump_command - + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + + ( + (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) + if defaults_file_descriptor + else () + ) + (tuple(database['options'].split(' ')) if 'options' in database else ()) + (('--add-drop-database',) if database.get('add_drop_database', True) else ()) + (('--host', database['hostname']) if 'hostname' in database else ()) @@ -156,10 +176,16 @@ def dump_data_sources( for database in databases: dump_path = make_dump_path(borgmatic_runtime_directory) - username = borgmatic.hooks.credential.parse.resolve_credential(database.get('username'), config) - password = borgmatic.hooks.credential.parse.resolve_credential(database.get('password'), config) + username = borgmatic.hooks.credential.parse.resolve_credential( + database.get('username'), config + ) + password = borgmatic.hooks.credential.parse.resolve_credential( + database.get('password'), config + ) environment = dict(os.environ) - dump_database_names = database_names_to_dump(database, config, username, password, environment, dry_run) + dump_database_names = database_names_to_dump( + database, config, username, password, environment, dry_run + ) if not dump_database_names: if dry_run: @@ -282,10 +308,16 @@ def restore_data_source_dump( mysql_restore_command = tuple( shlex.quote(part) for part in shlex.split(data_source.get('mysql_command') or 'mysql') ) - defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe(username, password) + defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe( + username, password + ) restore_command = ( mysql_restore_command - + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + + ( + (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) + if defaults_file_descriptor + else () + ) + ('--batch',) + ( tuple(data_source['restore_options'].split(' ')) diff --git a/tests/unit/hooks/data_source/test_mariadb.py b/tests/unit/hooks/data_source/test_mariadb.py index 474c7d6d..6db363b8 100644 --- a/tests/unit/hooks/data_source/test_mariadb.py +++ b/tests/unit/hooks/data_source/test_mariadb.py @@ -9,7 +9,9 @@ from borgmatic.hooks.data_source import mariadb as module def test_database_names_to_dump_passes_through_name(): environment = flexmock() - names = module.database_names_to_dump({'name': 'foo'}, {}, environment, dry_run=False) + names = module.database_names_to_dump( + {'name': 'foo'}, {}, 'root', 'trustsome1', environment, dry_run=False + ) assert names == ('foo',) @@ -18,7 +20,9 @@ def test_database_names_to_dump_bails_for_dry_run(): environment = flexmock() flexmock(module).should_receive('execute_command_and_capture_output').never() - names = module.database_names_to_dump({'name': 'all'}, {}, environment, dry_run=True) + names = module.database_names_to_dump( + {'name': 'all'}, {}, 'root', 'trustsome1', environment, dry_run=True + ) assert names == () @@ -28,12 +32,24 @@ def test_database_names_to_dump_queries_mariadb_for_database_names(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module).should_receive('execute_command_and_capture_output').with_args( - ('mariadb', '--skip-column-names', '--batch', '--execute', 'show schemas'), + ( + 'mariadb', + '--defaults-extra-file=/dev/fd/99', + '--skip-column-names', + '--batch', + '--execute', + 'show schemas', + ), environment=environment, ).and_return('foo\nbar\nmysql\n').once() - names = module.database_names_to_dump({'name': 'all'}, {}, environment, dry_run=False) + names = module.database_names_to_dump( + {'name': 'all'}, {}, 'root', 'trustsome1', environment, dry_run=False + ) assert names == ('foo', 'bar') @@ -53,6 +69,9 @@ def test_dump_data_sources_dumps_each_database(): databases = [{'name': 'foo'}, {'name': 'bar'}] processes = [flexmock(), flexmock()] flexmock(module).should_receive('make_dump_path').and_return('') + flexmock(module.borgmatic.hooks.credential.parse).should_receive( + 'resolve_credential' + ).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' @@ -65,6 +84,8 @@ def test_dump_data_sources_dumps_each_database(): flexmock(module).should_receive('execute_dump_command').with_args( database={'name': name}, config={}, + username=None, + password=None, dump_path=object, database_names=(name,), environment={'USER': 'root'}, @@ -89,10 +110,10 @@ def test_dump_data_sources_dumps_with_password(): database = {'name': 'foo', 'username': 'root', 'password': 'trustsome1'} process = flexmock() flexmock(module).should_receive('make_dump_path').and_return('') - flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('database_names_to_dump').and_return(('foo',)).and_return( ('bar',) ) @@ -100,9 +121,11 @@ def test_dump_data_sources_dumps_with_password(): flexmock(module).should_receive('execute_dump_command').with_args( database=database, config={}, + username='root', + password='trustsome1', dump_path=object, database_names=('foo',), - environment={'USER': 'root', 'MYSQL_PWD': 'trustsome1'}, + environment={'USER': 'root'}, dry_run=object, dry_run_label=object, ).and_return(process).once() @@ -121,14 +144,16 @@ def test_dump_data_sources_dumps_all_databases_at_once(): databases = [{'name': 'all'}] process = flexmock() flexmock(module).should_receive('make_dump_path').and_return('') - flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('database_names_to_dump').and_return(('foo', 'bar')) flexmock(module).should_receive('execute_dump_command').with_args( database={'name': 'all'}, config={}, + username=None, + password=None, dump_path=object, database_names=('foo', 'bar'), environment={'USER': 'root'}, @@ -150,16 +175,18 @@ def test_dump_data_sources_dumps_all_databases_separately_when_format_configured databases = [{'name': 'all', 'format': 'sql'}] processes = [flexmock(), flexmock()] flexmock(module).should_receive('make_dump_path').and_return('') - flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' - ).replace_with(lambda value, config: value) + ).and_return(None) + flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('database_names_to_dump').and_return(('foo', 'bar')) for name, process in zip(('foo', 'bar'), processes): flexmock(module).should_receive('execute_dump_command').with_args( database={'name': name, 'format': 'sql'}, config={}, + username=None, + password=None, dump_path=object, database_names=(name,), environment={'USER': 'root'}, @@ -181,11 +208,15 @@ def test_dump_data_sources_dumps_all_databases_separately_when_format_configured def test_database_names_to_dump_runs_mariadb_with_list_options(): - database = {'name': 'all', 'list_options': '--defaults-extra-file=mariadb.cnf'} + database = {'name': 'all', 'list_options': '--defaults-file=mariadb.cnf'} + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module).should_receive('execute_command_and_capture_output').with_args( ( 'mariadb', - '--defaults-extra-file=mariadb.cnf', + '--defaults-extra-file=/dev/fd/99', + '--defaults-file=mariadb.cnf', '--skip-column-names', '--batch', '--execute', @@ -194,20 +225,27 @@ def test_database_names_to_dump_runs_mariadb_with_list_options(): environment=None, ).and_return(('foo\nbar')).once() - assert module.database_names_to_dump(database, {}, None, '') == ('foo', 'bar') + assert module.database_names_to_dump(database, {}, 'root', 'trustsome1', None, '') == ( + 'foo', + 'bar', + ) def test_database_names_to_dump_runs_non_default_mariadb_with_list_options(): database = { 'name': 'all', - 'list_options': '--defaults-extra-file=mariadb.cnf', + 'list_options': '--defaults-file=mariadb.cnf', 'mariadb_command': 'custom_mariadb', } + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module).should_receive('execute_command_and_capture_output').with_args( environment=None, full_command=( 'custom_mariadb', # Custom MariaDB command - '--defaults-extra-file=mariadb.cnf', + '--defaults-extra-file=/dev/fd/99', + '--defaults-file=mariadb.cnf', '--skip-column-names', '--batch', '--execute', @@ -215,7 +253,10 @@ def test_database_names_to_dump_runs_non_default_mariadb_with_list_options(): ), ).and_return(('foo\nbar')).once() - assert module.database_names_to_dump(database, {}, None, '') == ('foo', 'bar') + assert module.database_names_to_dump(database, {}, 'root', 'trustsome1', None, '') == ( + 'foo', + 'bar', + ) def test_execute_dump_command_runs_mariadb_dump(): @@ -225,11 +266,15 @@ def test_execute_dump_command_runs_mariadb_dump(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mariadb-dump', + '--defaults-extra-file=/dev/fd/99', '--add-drop-database', '--databases', 'foo', @@ -244,6 +289,8 @@ def test_execute_dump_command_runs_mariadb_dump(): module.execute_dump_command( database={'name': 'foo'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -261,11 +308,15 @@ def test_execute_dump_command_runs_mariadb_dump_without_add_drop_database(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mariadb-dump', + '--defaults-extra-file=/dev/fd/99', '--databases', 'foo', '--result-file', @@ -279,6 +330,8 @@ def test_execute_dump_command_runs_mariadb_dump_without_add_drop_database(): module.execute_dump_command( database={'name': 'foo', 'add_drop_database': False}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -296,11 +349,15 @@ def test_execute_dump_command_runs_mariadb_dump_with_hostname_and_port(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mariadb-dump', + '--defaults-extra-file=/dev/fd/99', '--add-drop-database', '--host', 'database.example.org', @@ -321,6 +378,8 @@ def test_execute_dump_command_runs_mariadb_dump_with_hostname_and_port(): module.execute_dump_command( database={'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -338,20 +397,22 @@ def test_execute_dump_command_runs_mariadb_dump_with_username_and_password(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mariadb-dump', + '--defaults-extra-file=/dev/fd/99', '--add-drop-database', - '--user', - 'root', '--databases', 'foo', '--result-file', 'dump', ), - environment={'MYSQL_PWD': 'trustsome1'}, + environment={}, run_to_completion=False, ).and_return(process).once() @@ -359,9 +420,11 @@ def test_execute_dump_command_runs_mariadb_dump_with_username_and_password(): module.execute_dump_command( database={'name': 'foo', 'username': 'root', 'password': 'trustsome1'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), - environment={'MYSQL_PWD': 'trustsome1'}, + environment={}, dry_run=False, dry_run_label='', ) @@ -376,11 +439,15 @@ def test_execute_dump_command_runs_mariadb_dump_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mariadb-dump', + '--defaults-extra-file=/dev/fd/99', '--stuff=such', '--add-drop-database', '--databases', @@ -396,6 +463,8 @@ def test_execute_dump_command_runs_mariadb_dump_with_options(): module.execute_dump_command( database={'name': 'foo', 'options': '--stuff=such'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -413,11 +482,15 @@ def test_execute_dump_command_runs_non_default_mariadb_dump_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'custom_mariadb_dump', # Custom MariaDB dump command + '--defaults-extra-file=/dev/fd/99', '--stuff=such', '--add-drop-database', '--databases', @@ -437,6 +510,8 @@ def test_execute_dump_command_runs_non_default_mariadb_dump_with_options(): 'options': '--stuff=such', }, # Custom MariaDB dump command specified config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -450,6 +525,9 @@ def test_execute_dump_command_runs_non_default_mariadb_dump_with_options(): def test_execute_dump_command_with_duplicate_dump_skips_mariadb_dump(): flexmock(module.dump).should_receive('make_data_source_dump_filename').and_return('dump') flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump').never() flexmock(module).should_receive('execute_command').never() @@ -457,6 +535,8 @@ def test_execute_dump_command_with_duplicate_dump_skips_mariadb_dump(): module.execute_dump_command( database={'name': 'foo'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -473,6 +553,9 @@ def test_execute_dump_command_with_dry_run_skips_mariadb_dump(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').never() @@ -481,6 +564,8 @@ def test_execute_dump_command_with_dry_run_skips_mariadb_dump(): module.execute_dump_command( database={'name': 'foo'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -685,13 +770,16 @@ def test_restore_data_source_dump_runs_mariadb_with_username_and_password(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( - ('mariadb', '--batch', '--user', 'root'), + ('mariadb', '--defaults-extra-file=/dev/fd/99', '--batch'), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - environment={'USER': 'root', 'MYSQL_PWD': 'trustsome1'}, + environment={'USER': 'root'}, ).once() module.restore_data_source_dump( @@ -727,10 +815,14 @@ def test_restore_data_source_dump_with_connection_params_uses_connection_params_ flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'cliusername', 'clipassword' + ).and_return(99) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( 'mariadb', + '--defaults-extra-file=/dev/fd/99', '--batch', '--host', 'clihost', @@ -738,13 +830,11 @@ def test_restore_data_source_dump_with_connection_params_uses_connection_params_ 'cliport', '--protocol', 'tcp', - '--user', - 'cliusername', ), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - environment={'USER': 'root', 'MYSQL_PWD': 'clipassword'}, + environment={'USER': 'root'}, ).once() module.restore_data_source_dump( @@ -782,10 +872,14 @@ def test_restore_data_source_dump_without_connection_params_uses_restore_params_ flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'restoreuser', 'restorepass' + ).and_return(99) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( 'mariadb', + '--defaults-extra-file=/dev/fd/99', '--batch', '--host', 'restorehost', @@ -793,13 +887,11 @@ def test_restore_data_source_dump_without_connection_params_uses_restore_params_ 'restoreport', '--protocol', 'tcp', - '--user', - 'restoreuser', ), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - environment={'USER': 'root', 'MYSQL_PWD': 'restorepass'}, + environment={'USER': 'root'}, ).once() module.restore_data_source_dump( diff --git a/tests/unit/hooks/data_source/test_mysql.py b/tests/unit/hooks/data_source/test_mysql.py index 5b4cbcc9..0fb3f432 100644 --- a/tests/unit/hooks/data_source/test_mysql.py +++ b/tests/unit/hooks/data_source/test_mysql.py @@ -9,7 +9,9 @@ from borgmatic.hooks.data_source import mysql as module def test_database_names_to_dump_passes_through_name(): environment = flexmock() - names = module.database_names_to_dump({'name': 'foo'}, {}, environment, dry_run=False) + names = module.database_names_to_dump( + {'name': 'foo'}, {}, 'root', 'trustsome1', environment, dry_run=False + ) assert names == ('foo',) @@ -21,7 +23,9 @@ def test_database_names_to_dump_bails_for_dry_run(): ).replace_with(lambda value, config: value) flexmock(module).should_receive('execute_command_and_capture_output').never() - names = module.database_names_to_dump({'name': 'all'}, {}, environment, dry_run=True) + names = module.database_names_to_dump( + {'name': 'all'}, {}, 'root', 'trustsome1', environment, dry_run=True + ) assert names == () @@ -31,12 +35,24 @@ def test_database_names_to_dump_queries_mysql_for_database_names(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module).should_receive('execute_command_and_capture_output').with_args( - ('mysql', '--skip-column-names', '--batch', '--execute', 'show schemas'), + ( + 'mysql', + '--defaults-extra-file=/dev/fd/99', + '--skip-column-names', + '--batch', + '--execute', + 'show schemas', + ), environment=environment, ).and_return('foo\nbar\nmysql\n').once() - names = module.database_names_to_dump({'name': 'all'}, {}, environment, dry_run=False) + names = module.database_names_to_dump( + {'name': 'all'}, {}, 'root', 'trustsome1', environment, dry_run=False + ) assert names == ('foo', 'bar') @@ -56,6 +72,9 @@ def test_dump_data_sources_dumps_each_database(): databases = [{'name': 'foo'}, {'name': 'bar'}] processes = [flexmock(), flexmock()] flexmock(module).should_receive('make_dump_path').and_return('') + flexmock(module.borgmatic.hooks.credential.parse).should_receive( + 'resolve_credential' + ).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('database_names_to_dump').and_return(('foo',)).and_return( ('bar',) @@ -65,6 +84,8 @@ def test_dump_data_sources_dumps_each_database(): flexmock(module).should_receive('execute_dump_command').with_args( database={'name': name}, config={}, + username=None, + password=None, dump_path=object, database_names=(name,), environment={'USER': 'root'}, @@ -89,10 +110,10 @@ def test_dump_data_sources_dumps_with_password(): database = {'name': 'foo', 'username': 'root', 'password': 'trustsome1'} process = flexmock() flexmock(module).should_receive('make_dump_path').and_return('') - flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('database_names_to_dump').and_return(('foo',)).and_return( ('bar',) ) @@ -100,9 +121,11 @@ def test_dump_data_sources_dumps_with_password(): flexmock(module).should_receive('execute_dump_command').with_args( database=database, config={}, + username='root', + password='trustsome1', dump_path=object, database_names=('foo',), - environment={'USER': 'root', 'MYSQL_PWD': 'trustsome1'}, + environment={'USER': 'root'}, dry_run=object, dry_run_label=object, ).and_return(process).once() @@ -121,11 +144,16 @@ def test_dump_data_sources_dumps_all_databases_at_once(): databases = [{'name': 'all'}] process = flexmock() flexmock(module).should_receive('make_dump_path').and_return('') + flexmock(module.borgmatic.hooks.credential.parse).should_receive( + 'resolve_credential' + ).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('database_names_to_dump').and_return(('foo', 'bar')) flexmock(module).should_receive('execute_dump_command').with_args( database={'name': 'all'}, config={}, + username=None, + password=None, dump_path=object, database_names=('foo', 'bar'), environment={'USER': 'root'}, @@ -147,6 +175,9 @@ def test_dump_data_sources_dumps_all_databases_separately_when_format_configured databases = [{'name': 'all', 'format': 'sql'}] processes = [flexmock(), flexmock()] flexmock(module).should_receive('make_dump_path').and_return('') + flexmock(module.borgmatic.hooks.credential.parse).should_receive( + 'resolve_credential' + ).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('database_names_to_dump').and_return(('foo', 'bar')) @@ -154,6 +185,8 @@ def test_dump_data_sources_dumps_all_databases_separately_when_format_configured flexmock(module).should_receive('execute_dump_command').with_args( database={'name': name, 'format': 'sql'}, config={}, + username=None, + password=None, dump_path=object, database_names=(name,), environment={'USER': 'root'}, @@ -175,11 +208,15 @@ def test_dump_data_sources_dumps_all_databases_separately_when_format_configured def test_database_names_to_dump_runs_mysql_with_list_options(): - database = {'name': 'all', 'list_options': '--defaults-extra-file=my.cnf'} + database = {'name': 'all', 'list_options': '--defaults-file=my.cnf'} + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module).should_receive('execute_command_and_capture_output').with_args( ( 'mysql', - '--defaults-extra-file=my.cnf', + '--defaults-extra-file=/dev/fd/99', + '--defaults-file=my.cnf', '--skip-column-names', '--batch', '--execute', @@ -188,20 +225,27 @@ def test_database_names_to_dump_runs_mysql_with_list_options(): environment=None, ).and_return(('foo\nbar')).once() - assert module.database_names_to_dump(database, {}, None, '') == ('foo', 'bar') + assert module.database_names_to_dump(database, {}, 'root', 'trustsome1', None, '') == ( + 'foo', + 'bar', + ) def test_database_names_to_dump_runs_non_default_mysql_with_list_options(): database = { 'name': 'all', - 'list_options': '--defaults-extra-file=my.cnf', + 'list_options': '--defaults-file=my.cnf', 'mysql_command': 'custom_mysql', } + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module).should_receive('execute_command_and_capture_output').with_args( environment=None, full_command=( 'custom_mysql', # Custom MySQL command - '--defaults-extra-file=my.cnf', + '--defaults-extra-file=/dev/fd/99', + '--defaults-file=my.cnf', '--skip-column-names', '--batch', '--execute', @@ -209,7 +253,10 @@ def test_database_names_to_dump_runs_non_default_mysql_with_list_options(): ), ).and_return(('foo\nbar')).once() - assert module.database_names_to_dump(database, {}, None, '') == ('foo', 'bar') + assert module.database_names_to_dump(database, {}, 'root', 'trustsome1', None, '') == ( + 'foo', + 'bar', + ) def test_execute_dump_command_runs_mysqldump(): @@ -219,11 +266,15 @@ def test_execute_dump_command_runs_mysqldump(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mysqldump', + '--defaults-extra-file=/dev/fd/99', '--add-drop-database', '--databases', 'foo', @@ -238,6 +289,8 @@ def test_execute_dump_command_runs_mysqldump(): module.execute_dump_command( database={'name': 'foo'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -255,11 +308,15 @@ def test_execute_dump_command_runs_mysqldump_without_add_drop_database(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mysqldump', + '--defaults-extra-file=/dev/fd/99', '--databases', 'foo', '--result-file', @@ -273,6 +330,8 @@ def test_execute_dump_command_runs_mysqldump_without_add_drop_database(): module.execute_dump_command( database={'name': 'foo', 'add_drop_database': False}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -290,11 +349,15 @@ def test_execute_dump_command_runs_mysqldump_with_hostname_and_port(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mysqldump', + '--defaults-extra-file=/dev/fd/99', '--add-drop-database', '--host', 'database.example.org', @@ -315,6 +378,8 @@ def test_execute_dump_command_runs_mysqldump_with_hostname_and_port(): module.execute_dump_command( database={'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -332,20 +397,22 @@ def test_execute_dump_command_runs_mysqldump_with_username_and_password(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mysqldump', + '--defaults-extra-file=/dev/fd/99', '--add-drop-database', - '--user', - 'root', '--databases', 'foo', '--result-file', 'dump', ), - environment={'MYSQL_PWD': 'trustsome1'}, + environment={}, run_to_completion=False, ).and_return(process).once() @@ -353,9 +420,11 @@ def test_execute_dump_command_runs_mysqldump_with_username_and_password(): module.execute_dump_command( database={'name': 'foo', 'username': 'root', 'password': 'trustsome1'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), - environment={'MYSQL_PWD': 'trustsome1'}, + environment={}, dry_run=False, dry_run_label='', ) @@ -370,11 +439,15 @@ def test_execute_dump_command_runs_mysqldump_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mysqldump', + '--defaults-extra-file=/dev/fd/99', '--stuff=such', '--add-drop-database', '--databases', @@ -390,6 +463,8 @@ def test_execute_dump_command_runs_mysqldump_with_options(): module.execute_dump_command( database={'name': 'foo', 'options': '--stuff=such'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -407,11 +482,15 @@ def test_execute_dump_command_runs_non_default_mysqldump(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'custom_mysqldump', # Custom MySQL dump command + '--defaults-extra-file=/dev/fd/99', '--add-drop-database', '--databases', 'foo', @@ -429,6 +508,8 @@ def test_execute_dump_command_runs_non_default_mysqldump(): 'mysql_dump_command': 'custom_mysqldump', }, # Custom MySQL dump command specified config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -442,6 +523,9 @@ def test_execute_dump_command_runs_non_default_mysqldump(): def test_execute_dump_command_with_duplicate_dump_skips_mysqldump(): flexmock(module.dump).should_receive('make_data_source_dump_filename').and_return('dump') flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump').never() flexmock(module).should_receive('execute_command').never() @@ -449,6 +533,8 @@ def test_execute_dump_command_with_duplicate_dump_skips_mysqldump(): module.execute_dump_command( database={'name': 'foo'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -465,6 +551,9 @@ def test_execute_dump_command_with_dry_run_skips_mysqldump(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').never() @@ -473,6 +562,8 @@ def test_execute_dump_command_with_dry_run_skips_mysqldump(): module.execute_dump_command( database={'name': 'foo'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -675,13 +766,16 @@ def test_restore_data_source_dump_runs_mysql_with_username_and_password(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( - ('mysql', '--batch', '--user', 'root'), + ('mysql', '--defaults-extra-file=/dev/fd/99', '--batch'), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - environment={'USER': 'root', 'MYSQL_PWD': 'trustsome1'}, + environment={'USER': 'root'}, ).once() module.restore_data_source_dump( @@ -717,10 +811,14 @@ def test_restore_data_source_dump_with_connection_params_uses_connection_params_ flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('cliusername', 'clipassword').and_return(99) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( 'mysql', + '--defaults-extra-file=/dev/fd/99', '--batch', '--host', 'clihost', @@ -728,13 +826,11 @@ def test_restore_data_source_dump_with_connection_params_uses_connection_params_ 'cliport', '--protocol', 'tcp', - '--user', - 'cliusername', ), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - environment={'USER': 'root', 'MYSQL_PWD': 'clipassword'}, + environment={'USER': 'root'}, ).once() module.restore_data_source_dump( @@ -772,10 +868,14 @@ def test_restore_data_source_dump_without_connection_params_uses_restore_params_ flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('restoreuser', 'restorepass').and_return(99) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( 'mysql', + '--defaults-extra-file=/dev/fd/99', '--batch', '--host', 'restorehost', @@ -783,13 +883,11 @@ def test_restore_data_source_dump_without_connection_params_uses_restore_params_ 'restoreport', '--protocol', 'tcp', - '--user', - 'restoreuser', ), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - environment={'USER': 'root', 'MYSQL_PWD': 'restorepass'}, + environment={'USER': 'root'}, ).once() module.restore_data_source_dump( diff --git a/tests/unit/test_execute.py b/tests/unit/test_execute.py index 4d39fe3b..f3ef9413 100644 --- a/tests/unit/test_execute.py +++ b/tests/unit/test_execute.py @@ -191,7 +191,7 @@ def test_execute_command_calls_full_command(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -213,7 +213,7 @@ def test_execute_command_calls_full_command_with_output_file(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stderr=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -234,7 +234,7 @@ def test_execute_command_calls_full_command_without_capturing_output(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(wait=lambda: 0)).once() flexmock(module).should_receive('interpret_exit_code').and_return(module.Exit_status.SUCCESS) flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) @@ -257,7 +257,7 @@ def test_execute_command_calls_full_command_with_input_file(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -278,7 +278,7 @@ def test_execute_command_calls_full_command_with_shell(): shell=True, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -299,7 +299,7 @@ def test_execute_command_calls_full_command_with_environment(): shell=False, env={'a': 'b'}, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -320,33 +320,12 @@ def test_execute_command_calls_full_command_with_working_directory(): shell=False, env=None, cwd='/working', - close_fds=True, - ).and_return(flexmock(stdout=None)).once() - flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) - flexmock(module).should_receive('log_outputs') - - output = module.execute_command(full_command, working_directory='/working') - - assert output is None - - -def test_execute_command_with_BORG_PASSPHRASE_FD_leaves_file_descriptors_open(): - full_command = ['foo', 'bar'] - flexmock(module).should_receive('log_command') - flexmock(module.subprocess).should_receive('Popen').with_args( - full_command, - stdin=None, - stdout=module.subprocess.PIPE, - stderr=module.subprocess.STDOUT, - shell=False, - env={'BORG_PASSPHRASE_FD': '4'}, - cwd=None, close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') - output = module.execute_command(full_command, environment={'BORG_PASSPHRASE_FD': '4'}) + output = module.execute_command(full_command, working_directory='/working') assert output is None @@ -363,7 +342,7 @@ def test_execute_command_without_run_to_completion_returns_process(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(process).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -377,11 +356,12 @@ def test_execute_command_and_capture_output_returns_stdout(): flexmock(module).should_receive('log_command') flexmock(module.subprocess).should_receive('check_output').with_args( full_command, + stdin=None, stderr=None, shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(decode=lambda: expected_output)).once() output = module.execute_command_and_capture_output(full_command) @@ -395,11 +375,12 @@ def test_execute_command_and_capture_output_with_capture_stderr_returns_stderr() flexmock(module).should_receive('log_command') flexmock(module.subprocess).should_receive('check_output').with_args( full_command, + stdin=None, stderr=module.subprocess.STDOUT, shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(decode=lambda: expected_output)).once() output = module.execute_command_and_capture_output(full_command, capture_stderr=True) @@ -414,11 +395,12 @@ def test_execute_command_and_capture_output_returns_output_when_process_error_is flexmock(module).should_receive('log_command') flexmock(module.subprocess).should_receive('check_output').with_args( full_command, + stdin=None, stderr=None, shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_raise(subprocess.CalledProcessError(1, full_command, err_output)).once() flexmock(module).should_receive('interpret_exit_code').and_return( module.Exit_status.SUCCESS @@ -435,11 +417,12 @@ def test_execute_command_and_capture_output_raises_when_command_errors(): flexmock(module).should_receive('log_command') flexmock(module.subprocess).should_receive('check_output').with_args( full_command, + stdin=None, stderr=None, shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_raise(subprocess.CalledProcessError(2, full_command, expected_output)).once() flexmock(module).should_receive('interpret_exit_code').and_return( module.Exit_status.ERROR @@ -455,11 +438,12 @@ def test_execute_command_and_capture_output_returns_output_with_shell(): flexmock(module).should_receive('log_command') flexmock(module.subprocess).should_receive('check_output').with_args( 'foo bar', + stdin=None, stderr=None, shell=True, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(decode=lambda: expected_output)).once() output = module.execute_command_and_capture_output(full_command, shell=True) @@ -473,11 +457,12 @@ def test_execute_command_and_capture_output_returns_output_with_environment(): flexmock(module).should_receive('log_command') flexmock(module.subprocess).should_receive('check_output').with_args( full_command, + stdin=None, stderr=None, shell=False, env={'a': 'b'}, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(decode=lambda: expected_output)).once() output = module.execute_command_and_capture_output( @@ -493,37 +478,16 @@ def test_execute_command_and_capture_output_returns_output_with_working_director flexmock(module).should_receive('log_command') flexmock(module.subprocess).should_receive('check_output').with_args( full_command, + stdin=None, stderr=None, shell=False, env=None, cwd='/working', - close_fds=True, - ).and_return(flexmock(decode=lambda: expected_output)).once() - - output = module.execute_command_and_capture_output( - full_command, shell=False, working_directory='/working' - ) - - assert output == expected_output - - -def test_execute_command_and_capture_output_with_BORG_PASSPHRASE_FD_leaves_file_descriptors_open(): - full_command = ['foo', 'bar'] - expected_output = '[]' - flexmock(module).should_receive('log_command') - flexmock(module.subprocess).should_receive('check_output').with_args( - full_command, - stderr=None, - shell=False, - env={'BORG_PASSPHRASE_FD': '4'}, - cwd=None, close_fds=False, ).and_return(flexmock(decode=lambda: expected_output)).once() output = module.execute_command_and_capture_output( - full_command, - shell=False, - environment={'BORG_PASSPHRASE_FD': '4'}, + full_command, shell=False, working_directory='/working' ) assert output == expected_output @@ -541,7 +505,7 @@ def test_execute_command_with_processes_calls_full_command(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -564,7 +528,7 @@ def test_execute_command_with_processes_returns_output_with_output_log_level_non shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(process).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs').and_return({process: 'out'}) @@ -587,7 +551,7 @@ def test_execute_command_with_processes_calls_full_command_with_output_file(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stderr=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -609,7 +573,7 @@ def test_execute_command_with_processes_calls_full_command_without_capturing_out shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(wait=lambda: 0)).once() flexmock(module).should_receive('interpret_exit_code').and_return(module.Exit_status.SUCCESS) flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) @@ -635,7 +599,7 @@ def test_execute_command_with_processes_calls_full_command_with_input_file(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -657,7 +621,7 @@ def test_execute_command_with_processes_calls_full_command_with_shell(): shell=True, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -679,7 +643,7 @@ def test_execute_command_with_processes_calls_full_command_with_environment(): shell=False, env={'a': 'b'}, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -701,39 +665,13 @@ def test_execute_command_with_processes_calls_full_command_with_working_director shell=False, env=None, cwd='/working', - close_fds=True, - ).and_return(flexmock(stdout=None)).once() - flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) - flexmock(module).should_receive('log_outputs') - - output = module.execute_command_with_processes( - full_command, processes, working_directory='/working' - ) - - assert output is None - - -def test_execute_command_with_processes_with_BORG_PASSPHRASE_FD_leaves_file_descriptors_open(): - full_command = ['foo', 'bar'] - processes = (flexmock(),) - flexmock(module).should_receive('log_command') - flexmock(module.subprocess).should_receive('Popen').with_args( - full_command, - stdin=None, - stdout=module.subprocess.PIPE, - stderr=module.subprocess.STDOUT, - shell=False, - env={'BORG_PASSPHRASE_FD': '4'}, - cwd=None, close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') output = module.execute_command_with_processes( - full_command, - processes, - environment={'BORG_PASSPHRASE_FD': '4'}, + full_command, processes, working_directory='/working' ) assert output is None @@ -754,7 +692,7 @@ def test_execute_command_with_processes_kills_processes_on_error(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_raise(subprocess.CalledProcessError(1, full_command, 'error')).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs').never() From 1e274d7153b6ddce4710c3425d0a94f6587e4736 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 28 Feb 2025 08:59:38 -0800 Subject: [PATCH 4/9] Add some missing test mocking (#1009). --- tests/unit/hooks/data_source/test_mariadb.py | 15 +++++++++++++++ tests/unit/hooks/data_source/test_mysql.py | 15 +++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/tests/unit/hooks/data_source/test_mariadb.py b/tests/unit/hooks/data_source/test_mariadb.py index 6db363b8..c9c675b1 100644 --- a/tests/unit/hooks/data_source/test_mariadb.py +++ b/tests/unit/hooks/data_source/test_mariadb.py @@ -631,6 +631,9 @@ def test_restore_data_source_dump_runs_mariadb_to_restore(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive( + 'make_defaults_file_pipe' + ).with_args(None, None).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('mariadb', '--batch'), @@ -663,6 +666,9 @@ def test_restore_data_source_dump_runs_mariadb_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive( + 'make_defaults_file_pipe' + ).with_args(None, None).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('mariadb', '--batch', '--harder'), @@ -697,6 +703,9 @@ def test_restore_data_source_dump_runs_non_default_mariadb_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive( + 'make_defaults_file_pipe' + ).with_args(None, None).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('custom_mariadb', '--batch', '--harder'), @@ -729,6 +738,9 @@ def test_restore_data_source_dump_runs_mariadb_with_hostname_and_port(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive( + 'make_defaults_file_pipe' + ).with_args(None, None).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( @@ -916,6 +928,9 @@ def test_restore_data_source_dump_with_dry_run_skips_restore(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive( + 'make_defaults_file_pipe' + ).with_args(None, None).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').never() diff --git a/tests/unit/hooks/data_source/test_mysql.py b/tests/unit/hooks/data_source/test_mysql.py index 0fb3f432..c7098e4a 100644 --- a/tests/unit/hooks/data_source/test_mysql.py +++ b/tests/unit/hooks/data_source/test_mysql.py @@ -629,6 +629,9 @@ def test_restore_data_source_dump_runs_mysql_to_restore(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args(None, None).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('mysql', '--batch'), @@ -661,6 +664,9 @@ def test_restore_data_source_dump_runs_mysql_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args(None, None).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('mysql', '--batch', '--harder'), @@ -693,6 +699,9 @@ def test_restore_data_source_dump_runs_non_default_mysql_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args(None, None).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('custom_mysql', '--batch', '--harder'), @@ -725,6 +734,9 @@ def test_restore_data_source_dump_runs_mysql_with_hostname_and_port(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args(None, None).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( @@ -912,6 +924,9 @@ def test_restore_data_source_dump_with_dry_run_skips_restore(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args(None, None).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').never() From 48a4fbaa89b443ac0871a639c674e50b23e5a7ae Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 28 Feb 2025 09:21:01 -0800 Subject: [PATCH 5/9] Add missing test coverage for defaults file function (#1009). --- tests/unit/hooks/data_source/test_mariadb.py | 78 ++++++++++++++++---- 1 file changed, 63 insertions(+), 15 deletions(-) diff --git a/tests/unit/hooks/data_source/test_mariadb.py b/tests/unit/hooks/data_source/test_mariadb.py index c9c675b1..dc2c1a28 100644 --- a/tests/unit/hooks/data_source/test_mariadb.py +++ b/tests/unit/hooks/data_source/test_mariadb.py @@ -6,6 +6,54 @@ from flexmock import flexmock from borgmatic.hooks.data_source import mariadb as module +def test_make_defaults_file_pipe_without_username_or_password_bails(): + flexmock(module.os).should_receive('pipe').never() + + assert module.make_defaults_file_pipe(username=None, password=None) is None + + +def test_make_defaults_file_pipe_with_username_and_password_writes_them_to_file_descriptor(): + read_descriptor = flexmock() + write_descriptor = flexmock() + + flexmock(module.os).should_receive('pipe').and_return(read_descriptor, write_descriptor) + flexmock(module.os).should_receive('write').with_args( + write_descriptor, b'[client]\nuser=root\npassword=trustsome1' + ).once() + flexmock(module.os).should_receive('close') + flexmock(module.os).should_receive('set_inheritable') + + assert module.make_defaults_file_pipe(username='root', password='trustsome1') == read_descriptor + + +def test_make_defaults_file_pipe_with_username_only_writes_it_to_file_descriptor(): + read_descriptor = flexmock() + write_descriptor = flexmock() + + flexmock(module.os).should_receive('pipe').and_return(read_descriptor, write_descriptor) + flexmock(module.os).should_receive('write').with_args( + write_descriptor, b'[client]\nuser=root' + ).once() + flexmock(module.os).should_receive('close') + flexmock(module.os).should_receive('set_inheritable') + + assert module.make_defaults_file_pipe(username='root', password=None) == read_descriptor + + +def test_make_defaults_file_pipe_with_password_only_writes_it_to_file_descriptor(): + read_descriptor = flexmock() + write_descriptor = flexmock() + + flexmock(module.os).should_receive('pipe').and_return(read_descriptor, write_descriptor) + flexmock(module.os).should_receive('write').with_args( + write_descriptor, b'[client]\npassword=trustsome1' + ).once() + flexmock(module.os).should_receive('close') + flexmock(module.os).should_receive('set_inheritable') + + assert module.make_defaults_file_pipe(username=None, password='trustsome1') == read_descriptor + + def test_database_names_to_dump_passes_through_name(): environment = flexmock() @@ -631,9 +679,9 @@ def test_restore_data_source_dump_runs_mariadb_to_restore(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'make_defaults_file_pipe' - ).with_args(None, None).and_return(None) + flexmock(module).should_receive('make_defaults_file_pipe').with_args(None, None).and_return( + None + ) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('mariadb', '--batch'), @@ -666,9 +714,9 @@ def test_restore_data_source_dump_runs_mariadb_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'make_defaults_file_pipe' - ).with_args(None, None).and_return(None) + flexmock(module).should_receive('make_defaults_file_pipe').with_args(None, None).and_return( + None + ) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('mariadb', '--batch', '--harder'), @@ -703,9 +751,9 @@ def test_restore_data_source_dump_runs_non_default_mariadb_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'make_defaults_file_pipe' - ).with_args(None, None).and_return(None) + flexmock(module).should_receive('make_defaults_file_pipe').with_args(None, None).and_return( + None + ) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('custom_mariadb', '--batch', '--harder'), @@ -738,9 +786,9 @@ def test_restore_data_source_dump_runs_mariadb_with_hostname_and_port(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'make_defaults_file_pipe' - ).with_args(None, None).and_return(None) + flexmock(module).should_receive('make_defaults_file_pipe').with_args(None, None).and_return( + None + ) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( @@ -928,9 +976,9 @@ def test_restore_data_source_dump_with_dry_run_skips_restore(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'make_defaults_file_pipe' - ).with_args(None, None).and_return(None) + flexmock(module).should_receive('make_defaults_file_pipe').with_args(None, None).and_return( + None + ) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').never() From baf5fec78d5185a43738eda498575ca2d2347d36 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 28 Feb 2025 10:53:17 -0800 Subject: [PATCH 6/9] If the user supplies their own --defaults-extra-file, include it from the one we generate (#1009). --- borgmatic/hooks/data_source/mariadb.py | 152 +++++++++++++++++++------ borgmatic/hooks/data_source/mysql.py | 40 +++---- 2 files changed, 131 insertions(+), 61 deletions(-) diff --git a/borgmatic/hooks/data_source/mariadb.py b/borgmatic/hooks/data_source/mariadb.py index c57abf7a..4653319a 100644 --- a/borgmatic/hooks/data_source/mariadb.py +++ b/borgmatic/hooks/data_source/mariadb.py @@ -1,6 +1,7 @@ import copy import logging import os +import re import shlex import borgmatic.borg.pattern @@ -23,19 +24,46 @@ def make_dump_path(base_directory): # pragma: no cover return dump.make_data_source_dump_path(base_directory, 'mariadb_databases') -SYSTEM_DATABASE_NAMES = ('information_schema', 'mysql', 'performance_schema', 'sys') +DEFAULTS_EXTRA_FILE_FLAG_PATTERN = re.compile('^--defaults-extra-file=(?P.*)$') -def make_defaults_file_pipe(username=None, password=None): +def parse_extra_options(extra_options): ''' - Given a database username and/or password, write it to an anonymous pipe and return its file - descriptor for passing to an executed command. The idea is that this is a more secure way to - transmit credentials to a database client than using an environment variable. + Given an extra options string, split the options into a tuple and return it. Additionally, if + the first option is "--defaults-extra-file=...", then remove it from the options and return the + filename. - If no username or password are given, then return None. + So the return value is a tuple of: (parsed options, defaults extra filename). - Do not use this value for multiple different command invocations. That will not work because - each pipe is "used up" once read. + The intent is to support downstream merging of multiple "--defaults-extra-file"s, as + MariaDB/MySQL only allows one at a time. + ''' + split_extra_options = tuple(shlex.split(extra_options)) if extra_options else () + + if not split_extra_options: + return (split_extra_options, None) + + match = DEFAULTS_EXTRA_FILE_FLAG_PATTERN.match(split_extra_options[0]) + + if not match: + return (split_extra_options, None) + + return (split_extra_options[1:], match.group('filename')) + + +def make_defaults_file_options(username=None, password=None, defaults_extra_filename=None): + ''' + Given a database username and/or password, write it to an anonymous pipe and return the flags + for passing that file descriptor to an executed command. The idea is that this is a more secure + way to transmit credentials to a database client than using an environment variable. + + If no username or password are given, then return the options for the given defaults extra + filename (if any). But if there is a username and/or password and a defaults extra filename is + given, then "!include" it from the generated file, effectively allowing multiple defaults extra + files. + + Do not use the returned value for multiple different command invocations. That will not work + because each pipe is "used up" once read. ''' values = '\n'.join( ( @@ -45,7 +73,10 @@ def make_defaults_file_pipe(username=None, password=None): ).strip() if not values: - return None + if defaults_extra_filename: + return (f'--defaults-extra-file={defaults_extra_filename}',) + + return () fields_message = ' and '.join( field_name @@ -55,17 +86,20 @@ def make_defaults_file_pipe(username=None, password=None): ) if field_name is not None ) - logger.debug(f'Writing database {fields_message} to defaults extra file pipe') + include_message = f' (including {defaults_extra_filename})' if defaults_extra_filename else '' + logger.debug(f'Writing database {fields_message} to defaults extra file pipe{include_message}') + + include = f'!include {defaults_extra_filename}\n' if defaults_extra_filename else '' read_file_descriptor, write_file_descriptor = os.pipe() - os.write(write_file_descriptor, f'[client]\n{values}'.encode('utf-8')) + os.write(write_file_descriptor, f'{include}[client]\n{values}'.encode('utf-8')) os.close(write_file_descriptor) # This plus subprocess.Popen(..., close_fds=False) in execute.py is necessary for the database # client child process to inherit the file descriptor. os.set_inheritable(read_file_descriptor, True) - return read_file_descriptor + return (f'--defaults-extra-file=/dev/fd/{read_file_descriptor}',) def database_names_to_dump(database, config, username, password, environment, dry_run): @@ -83,15 +117,73 @@ def database_names_to_dump(database, config, username, password, environment, dr mariadb_show_command = tuple( shlex.quote(part) for part in shlex.split(database.get('mariadb_command') or 'mariadb') ) - defaults_file_descriptor = make_defaults_file_pipe(username, password) + extra_options, defaults_extra_filename = parse_extra_options(database.get('list_options')) show_command = ( mariadb_show_command - + ( - (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) - if defaults_file_descriptor - else () + + make_defaults_file_options(username, password, defaults_extra_filename) + + extra_options + + (('--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 ()) + + ('--skip-column-names', '--batch') + + ('--execute', 'show schemas') + ) + + logger.debug('Querying for "all" MariaDB databases to dump') + + show_output = execute_command_and_capture_output(show_command, environment=environment) + + return tuple( + show_name + for show_name in show_output.strip().splitlines() + if show_name not in SYSTEM_DATABASE_NAMES + ) + + +SYSTEM_DATABASE_NAMES = ('information_schema', 'mysql', 'performance_schema', 'sys') + + +def execute_dump_command( + database, + config, + username, + password, + dump_path, + database_names, + environment, + dry_run, + dry_run_label, +): + ''' + Kick off a dump for the given MariaDB database (provided as a configuration dict) to a named + pipe constructed from the given dump path and database name. + + Return a subprocess.Popen instance for the dump process ready to spew to a named pipe. But if + this is a dry run, then don't actually dump anything and return None. + ''' + database_name = database['name'] + dump_filename = dump.make_data_source_dump_filename( + dump_path, + database['name'], + database.get('hostname'), + database.get('port'), + ) + + if os.path.exists(dump_filename): + logger.warning( + f'Skipping duplicate dump of MariaDB database "{database_name}" to {dump_filename}' ) - + (tuple(database['list_options'].split(' ')) if 'list_options' in database else ()) + return None + + mariadb_dump_command = tuple( + shlex.quote(part) + for part in shlex.split(database.get('mariadb_dump_command') or 'mariadb-dump') + ) + extra_options, defaults_extra_filename = parse_extra_options(database.get('options')) + dump_command = ( + mariadb_dump_command + + make_defaults_file_options(username, password, defaults_extra_filename) + + extra_options + (('--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 ()) @@ -146,15 +238,11 @@ def execute_dump_command( shlex.quote(part) for part in shlex.split(database.get('mariadb_dump_command') or 'mariadb-dump') ) - defaults_file_descriptor = make_defaults_file_pipe(username, password) + extra_options, defaults_extra_filename = parse_extra_options(database.get('options')) dump_command = ( mariadb_dump_command - + ( - (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) - if defaults_file_descriptor - else () - ) - + (tuple(database['options'].split(' ')) if 'options' in database else ()) + + make_defaults_file_options(username, password, defaults_extra_filename) + + extra_options + (('--add-drop-database',) if database.get('add_drop_database', True) else ()) + (('--host', database['hostname']) if 'hostname' in database else ()) + (('--port', str(database['port'])) if 'port' in database else ()) @@ -346,20 +434,12 @@ def restore_data_source_dump( mariadb_restore_command = tuple( shlex.quote(part) for part in shlex.split(data_source.get('mariadb_command') or 'mariadb') ) - defaults_file_descriptor = make_defaults_file_pipe(username, password) + extra_options, defaults_extra_filename = parse_extra_options(database.get('restore_options')) restore_command = ( mariadb_restore_command - + ( - (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) - if defaults_file_descriptor - else () - ) + + make_defaults_file_options(username, password, defaults_extra_filename) + + extra_options + ('--batch',) - + ( - tuple(data_source['restore_options'].split(' ')) - if 'restore_options' in data_source - else () - ) + (('--host', hostname) if hostname else ()) + (('--port', str(port)) if port else ()) + (('--protocol', 'tcp') if hostname or port else ()) diff --git a/borgmatic/hooks/data_source/mysql.py b/borgmatic/hooks/data_source/mysql.py index a31f7a54..9c4685b7 100644 --- a/borgmatic/hooks/data_source/mysql.py +++ b/borgmatic/hooks/data_source/mysql.py @@ -42,17 +42,15 @@ def database_names_to_dump(database, config, username, password, environment, dr mysql_show_command = tuple( shlex.quote(part) for part in shlex.split(database.get('mysql_command') or 'mysql') ) - defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe( - username, password + extra_options, defaults_extra_filename = ( + borgmatic.hooks.data_source.mariadb.parse_extra_options(database.get('list_options')) ) show_command = ( mysql_show_command - + ( - (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) - if defaults_file_descriptor - else () + + borgmatic.hooks.data_source.mariadb.make_defaults_file_options( + username, password, defaults_extra_filename ) - + (tuple(database['list_options'].split(' ')) if 'list_options' in database else ()) + + extra_options + (('--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 ()) @@ -106,17 +104,15 @@ def execute_dump_command( mysql_dump_command = tuple( shlex.quote(part) for part in shlex.split(database.get('mysql_dump_command') or 'mysqldump') ) - defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe( - username, password + extra_options, defaults_extra_filename = ( + borgmatic.hooks.data_source.mariadb.parse_extra_options(database.get('options')) ) dump_command = ( mysql_dump_command - + ( - (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) - if defaults_file_descriptor - else () + + borgmatic.hooks.data_source.mariadb.make_defaults_file_options( + username, password, defaults_extra_filename ) - + (tuple(database['options'].split(' ')) if 'options' in database else ()) + + extra_options + (('--add-drop-database',) if database.get('add_drop_database', True) else ()) + (('--host', database['hostname']) if 'hostname' in database else ()) + (('--port', str(database['port'])) if 'port' in database else ()) @@ -308,22 +304,16 @@ def restore_data_source_dump( mysql_restore_command = tuple( shlex.quote(part) for part in shlex.split(data_source.get('mysql_command') or 'mysql') ) - defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe( - username, password + extra_options, defaults_extra_filename = ( + borgmatic.hooks.data_source.mariadb.parse_extra_options(database.get('restore_options')) ) restore_command = ( mysql_restore_command - + ( - (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) - if defaults_file_descriptor - else () + + borgmatic.hooks.data_source.mariadb.make_defaults_file_options( + username, password, defaults_extra_filename ) + + extra_options + ('--batch',) - + ( - tuple(data_source['restore_options'].split(' ')) - if 'restore_options' in data_source - else () - ) + (('--host', hostname) if hostname else ()) + (('--port', str(port)) if port else ()) + (('--protocol', 'tcp') if hostname or port else ()) From 1e5c256d54fa3edf88b80520c8df7049563a5f09 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 28 Feb 2025 14:40:00 -0800 Subject: [PATCH 7/9] Get tests passing again (#1009). --- borgmatic/hooks/data_source/mariadb.py | 4 +- borgmatic/hooks/data_source/mysql.py | 2 +- tests/unit/hooks/data_source/test_mariadb.py | 217 +++++++++++++------ tests/unit/hooks/data_source/test_mysql.py | 149 +++++++++---- 4 files changed, 254 insertions(+), 118 deletions(-) diff --git a/borgmatic/hooks/data_source/mariadb.py b/borgmatic/hooks/data_source/mariadb.py index 4653319a..985cd5b2 100644 --- a/borgmatic/hooks/data_source/mariadb.py +++ b/borgmatic/hooks/data_source/mariadb.py @@ -41,7 +41,7 @@ def parse_extra_options(extra_options): split_extra_options = tuple(shlex.split(extra_options)) if extra_options else () if not split_extra_options: - return (split_extra_options, None) + return ((), None) match = DEFAULTS_EXTRA_FILE_FLAG_PATTERN.match(split_extra_options[0]) @@ -434,7 +434,7 @@ def restore_data_source_dump( mariadb_restore_command = tuple( shlex.quote(part) for part in shlex.split(data_source.get('mariadb_command') or 'mariadb') ) - extra_options, defaults_extra_filename = parse_extra_options(database.get('restore_options')) + extra_options, defaults_extra_filename = parse_extra_options(data_source.get('restore_options')) restore_command = ( mariadb_restore_command + make_defaults_file_options(username, password, defaults_extra_filename) diff --git a/borgmatic/hooks/data_source/mysql.py b/borgmatic/hooks/data_source/mysql.py index 9c4685b7..13d77041 100644 --- a/borgmatic/hooks/data_source/mysql.py +++ b/borgmatic/hooks/data_source/mysql.py @@ -305,7 +305,7 @@ def restore_data_source_dump( shlex.quote(part) for part in shlex.split(data_source.get('mysql_command') or 'mysql') ) extra_options, defaults_extra_filename = ( - borgmatic.hooks.data_source.mariadb.parse_extra_options(database.get('restore_options')) + borgmatic.hooks.data_source.mariadb.parse_extra_options(data_source.get('restore_options')) ) restore_command = ( mysql_restore_command diff --git a/tests/unit/hooks/data_source/test_mariadb.py b/tests/unit/hooks/data_source/test_mariadb.py index dc2c1a28..d425b82c 100644 --- a/tests/unit/hooks/data_source/test_mariadb.py +++ b/tests/unit/hooks/data_source/test_mariadb.py @@ -9,11 +9,11 @@ from borgmatic.hooks.data_source import mariadb as module def test_make_defaults_file_pipe_without_username_or_password_bails(): flexmock(module.os).should_receive('pipe').never() - assert module.make_defaults_file_pipe(username=None, password=None) is None + assert module.make_defaults_file_options(username=None, password=None) is () -def test_make_defaults_file_pipe_with_username_and_password_writes_them_to_file_descriptor(): - read_descriptor = flexmock() +def test_make_defaults_file_option_with_username_and_password_writes_them_to_file_descriptor(): + read_descriptor = 99 write_descriptor = flexmock() flexmock(module.os).should_receive('pipe').and_return(read_descriptor, write_descriptor) @@ -23,11 +23,11 @@ def test_make_defaults_file_pipe_with_username_and_password_writes_them_to_file_ flexmock(module.os).should_receive('close') flexmock(module.os).should_receive('set_inheritable') - assert module.make_defaults_file_pipe(username='root', password='trustsome1') == read_descriptor + assert module.make_defaults_file_options(username='root', password='trustsome1') == ('--defaults-extra-file=/dev/fd/99',) def test_make_defaults_file_pipe_with_username_only_writes_it_to_file_descriptor(): - read_descriptor = flexmock() + read_descriptor = 99 write_descriptor = flexmock() flexmock(module.os).should_receive('pipe').and_return(read_descriptor, write_descriptor) @@ -37,11 +37,11 @@ def test_make_defaults_file_pipe_with_username_only_writes_it_to_file_descriptor flexmock(module.os).should_receive('close') flexmock(module.os).should_receive('set_inheritable') - assert module.make_defaults_file_pipe(username='root', password=None) == read_descriptor + assert module.make_defaults_file_options(username='root', password=None) == ('--defaults-extra-file=/dev/fd/99',) def test_make_defaults_file_pipe_with_password_only_writes_it_to_file_descriptor(): - read_descriptor = flexmock() + read_descriptor = 99 write_descriptor = flexmock() flexmock(module.os).should_receive('pipe').and_return(read_descriptor, write_descriptor) @@ -51,7 +51,21 @@ def test_make_defaults_file_pipe_with_password_only_writes_it_to_file_descriptor flexmock(module.os).should_receive('close') flexmock(module.os).should_receive('set_inheritable') - assert module.make_defaults_file_pipe(username=None, password='trustsome1') == read_descriptor + assert module.make_defaults_file_options(username=None, password='trustsome1') == ('--defaults-extra-file=/dev/fd/99',) + + +def test_make_defaults_file_option_with_defaults_extra_filename_includes_it_in_file_descriptor(): + read_descriptor = 99 + write_descriptor = flexmock() + + flexmock(module.os).should_receive('pipe').and_return(read_descriptor, write_descriptor) + flexmock(module.os).should_receive('write').with_args( + write_descriptor, b'!include extra.cnf\n[client]\nuser=root\npassword=trustsome1' + ).once() + flexmock(module.os).should_receive('close') + flexmock(module.os).should_receive('set_inheritable') + + assert module.make_defaults_file_options(username='root', password='trustsome1', defaults_extra_filename='extra.cnf') == ('--defaults-extra-file=/dev/fd/99',) def test_database_names_to_dump_passes_through_name(): @@ -80,9 +94,12 @@ def test_database_names_to_dump_queries_mariadb_for_database_names(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive('make_defaults_file_pipe').with_args( - 'root', 'trustsome1' - ).and_return(99) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return((), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module).should_receive('execute_command_and_capture_output').with_args( ( 'mariadb', @@ -256,15 +273,18 @@ def test_dump_data_sources_dumps_all_databases_separately_when_format_configured def test_database_names_to_dump_runs_mariadb_with_list_options(): - database = {'name': 'all', 'list_options': '--defaults-file=mariadb.cnf'} - flexmock(module).should_receive('make_defaults_file_pipe').with_args( - 'root', 'trustsome1' - ).and_return(99) + database = {'name': 'all', 'list_options': '--defaults-extra-file=mariadb.cnf --skip-ssl'} + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return(('--skip-ssl',), 'mariadb.cnf') + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', 'mariadb.cnf').and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module).should_receive('execute_command_and_capture_output').with_args( ( 'mariadb', '--defaults-extra-file=/dev/fd/99', - '--defaults-file=mariadb.cnf', + '--skip-ssl', '--skip-column-names', '--batch', '--execute', @@ -282,18 +302,21 @@ def test_database_names_to_dump_runs_mariadb_with_list_options(): def test_database_names_to_dump_runs_non_default_mariadb_with_list_options(): database = { 'name': 'all', - 'list_options': '--defaults-file=mariadb.cnf', + 'list_options': '--defaults-extra-file=mariadb.cnf --skip-ssl', 'mariadb_command': 'custom_mariadb', } - flexmock(module).should_receive('make_defaults_file_pipe').with_args( - 'root', 'trustsome1' - ).and_return(99) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return(('--skip-ssl',), 'mariadb.cnf') + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', 'mariadb.cnf').and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module).should_receive('execute_command_and_capture_output').with_args( environment=None, full_command=( 'custom_mariadb', # Custom MariaDB command '--defaults-extra-file=/dev/fd/99', - '--defaults-file=mariadb.cnf', + '--skip-ssl', '--skip-column-names', '--batch', '--execute', @@ -314,9 +337,12 @@ def test_execute_dump_command_runs_mariadb_dump(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive('make_defaults_file_pipe').with_args( - 'root', 'trustsome1' - ).and_return(99) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return((), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -356,9 +382,12 @@ def test_execute_dump_command_runs_mariadb_dump_without_add_drop_database(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive('make_defaults_file_pipe').with_args( - 'root', 'trustsome1' - ).and_return(99) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return((), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -397,9 +426,12 @@ def test_execute_dump_command_runs_mariadb_dump_with_hostname_and_port(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive('make_defaults_file_pipe').with_args( - 'root', 'trustsome1' - ).and_return(99) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return((), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -445,9 +477,12 @@ def test_execute_dump_command_runs_mariadb_dump_with_username_and_password(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive('make_defaults_file_pipe').with_args( - 'root', 'trustsome1' - ).and_return(99) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return((), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -487,9 +522,12 @@ def test_execute_dump_command_runs_mariadb_dump_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive('make_defaults_file_pipe').with_args( - 'root', 'trustsome1' - ).and_return(99) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return(('--stuff=such',), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -530,9 +568,12 @@ def test_execute_dump_command_runs_non_default_mariadb_dump_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive('make_defaults_file_pipe').with_args( - 'root', 'trustsome1' - ).and_return(99) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return(('--stuff=such',), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -573,9 +614,12 @@ def test_execute_dump_command_runs_non_default_mariadb_dump_with_options(): def test_execute_dump_command_with_duplicate_dump_skips_mariadb_dump(): flexmock(module.dump).should_receive('make_data_source_dump_filename').and_return('dump') flexmock(module.os.path).should_receive('exists').and_return(True) - flexmock(module).should_receive('make_defaults_file_pipe').with_args( - 'root', 'trustsome1' - ).and_return(99) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return((), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump').never() flexmock(module).should_receive('execute_command').never() @@ -601,9 +645,12 @@ def test_execute_dump_command_with_dry_run_skips_mariadb_dump(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive('make_defaults_file_pipe').with_args( - 'root', 'trustsome1' - ).and_return(99) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return((), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').never() @@ -679,9 +726,12 @@ def test_restore_data_source_dump_runs_mariadb_to_restore(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive('make_defaults_file_pipe').with_args(None, None).and_return( - None - ) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return((), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args(None, None, None).and_return(()) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('mariadb', '--batch'), @@ -714,12 +764,15 @@ def test_restore_data_source_dump_runs_mariadb_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive('make_defaults_file_pipe').with_args(None, None).and_return( - None - ) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return(('--harder',), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args(None, None, None).and_return(()) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( - ('mariadb', '--batch', '--harder'), + ('mariadb', '--harder', '--batch'), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, @@ -751,12 +804,15 @@ def test_restore_data_source_dump_runs_non_default_mariadb_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive('make_defaults_file_pipe').with_args(None, None).and_return( - None - ) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return(('--harder',), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args(None, None, None).and_return(()) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( - ('custom_mariadb', '--batch', '--harder'), + ('custom_mariadb', '--harder', '--batch'), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, @@ -786,9 +842,12 @@ def test_restore_data_source_dump_runs_mariadb_with_hostname_and_port(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive('make_defaults_file_pipe').with_args(None, None).and_return( - None - ) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return((), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args(None, None, None).and_return(()) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( @@ -830,9 +889,12 @@ def test_restore_data_source_dump_runs_mariadb_with_username_and_password(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive('make_defaults_file_pipe').with_args( - 'root', 'trustsome1' - ).and_return(99) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return((), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('mariadb', '--defaults-extra-file=/dev/fd/99', '--batch'), @@ -875,9 +937,14 @@ def test_restore_data_source_dump_with_connection_params_uses_connection_params_ flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive('make_defaults_file_pipe').with_args( - 'cliusername', 'clipassword' - ).and_return(99) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return((), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args('cliusername', 'clipassword', None).and_return( + ('--defaults-extra-file=/dev/fd/99',) + ) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( @@ -932,9 +999,14 @@ def test_restore_data_source_dump_without_connection_params_uses_restore_params_ flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive('make_defaults_file_pipe').with_args( - 'restoreuser', 'restorepass' - ).and_return(99) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return((), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args('restoreuser', 'restorepass', None).and_return( + ('--defaults-extra-file=/dev/fd/99',) + ) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( @@ -976,9 +1048,12 @@ def test_restore_data_source_dump_with_dry_run_skips_restore(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive('make_defaults_file_pipe').with_args(None, None).and_return( - None - ) + flexmock(module).should_receive( + 'parse_extra_options' + ).and_return((), None) + flexmock(module).should_receive( + 'make_defaults_file_options' + ).with_args(None, None, None).and_return(()) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').never() diff --git a/tests/unit/hooks/data_source/test_mysql.py b/tests/unit/hooks/data_source/test_mysql.py index c7098e4a..2e629dc3 100644 --- a/tests/unit/hooks/data_source/test_mysql.py +++ b/tests/unit/hooks/data_source/test_mysql.py @@ -36,8 +36,11 @@ def test_database_names_to_dump_queries_mysql_for_database_names(): 'resolve_credential' ).replace_with(lambda value, config: value) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args('root', 'trustsome1').and_return(99) + 'parse_extra_options' + ).and_return((), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module).should_receive('execute_command_and_capture_output').with_args( ( 'mysql', @@ -208,15 +211,18 @@ def test_dump_data_sources_dumps_all_databases_separately_when_format_configured def test_database_names_to_dump_runs_mysql_with_list_options(): - database = {'name': 'all', 'list_options': '--defaults-file=my.cnf'} + database = {'name': 'all', 'list_options': '--defaults-extra-file=my.cnf --skip-ssl'} flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args('root', 'trustsome1').and_return(99) + 'parse_extra_options' + ).and_return(('--skip-ssl',), 'my.cnf') + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', 'my.cnf').and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module).should_receive('execute_command_and_capture_output').with_args( ( 'mysql', '--defaults-extra-file=/dev/fd/99', - '--defaults-file=my.cnf', + '--skip-ssl', '--skip-column-names', '--batch', '--execute', @@ -234,18 +240,21 @@ def test_database_names_to_dump_runs_mysql_with_list_options(): def test_database_names_to_dump_runs_non_default_mysql_with_list_options(): database = { 'name': 'all', - 'list_options': '--defaults-file=my.cnf', + 'list_options': '--defaults-extra-file=my.cnf --skip-ssl', 'mysql_command': 'custom_mysql', } flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args('root', 'trustsome1').and_return(99) + 'parse_extra_options' + ).and_return(('--skip-ssl',), 'my.cnf') + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', 'my.cnf').and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module).should_receive('execute_command_and_capture_output').with_args( environment=None, full_command=( 'custom_mysql', # Custom MySQL command '--defaults-extra-file=/dev/fd/99', - '--defaults-file=my.cnf', + '--skip-ssl', '--skip-column-names', '--batch', '--execute', @@ -267,8 +276,11 @@ def test_execute_dump_command_runs_mysqldump(): 'resolve_credential' ).replace_with(lambda value, config: value) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args('root', 'trustsome1').and_return(99) + 'parse_extra_options' + ).and_return((), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -309,8 +321,11 @@ def test_execute_dump_command_runs_mysqldump_without_add_drop_database(): 'resolve_credential' ).replace_with(lambda value, config: value) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args('root', 'trustsome1').and_return(99) + 'parse_extra_options' + ).and_return((), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -350,8 +365,11 @@ def test_execute_dump_command_runs_mysqldump_with_hostname_and_port(): 'resolve_credential' ).replace_with(lambda value, config: value) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args('root', 'trustsome1').and_return(99) + 'parse_extra_options' + ).and_return((), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -398,8 +416,11 @@ def test_execute_dump_command_runs_mysqldump_with_username_and_password(): 'resolve_credential' ).replace_with(lambda value, config: value) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args('root', 'trustsome1').and_return(99) + 'parse_extra_options' + ).and_return((), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -440,8 +461,11 @@ def test_execute_dump_command_runs_mysqldump_with_options(): 'resolve_credential' ).replace_with(lambda value, config: value) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args('root', 'trustsome1').and_return(99) + 'parse_extra_options' + ).and_return(('--stuff=such',), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -483,8 +507,11 @@ def test_execute_dump_command_runs_non_default_mysqldump(): 'resolve_credential' ).replace_with(lambda value, config: value) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args('root', 'trustsome1').and_return(99) + 'parse_extra_options' + ).and_return((), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -524,8 +551,11 @@ def test_execute_dump_command_with_duplicate_dump_skips_mysqldump(): flexmock(module.dump).should_receive('make_data_source_dump_filename').and_return('dump') flexmock(module.os.path).should_receive('exists').and_return(True) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args('root', 'trustsome1').and_return(99) + 'parse_extra_options' + ).and_return((), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump').never() flexmock(module).should_receive('execute_command').never() @@ -552,8 +582,11 @@ def test_execute_dump_command_with_dry_run_skips_mysqldump(): 'resolve_credential' ).replace_with(lambda value, config: value) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args('root', 'trustsome1').and_return(99) + 'parse_extra_options' + ).and_return((), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').never() @@ -630,8 +663,11 @@ def test_restore_data_source_dump_runs_mysql_to_restore(): 'resolve_credential' ).replace_with(lambda value, config: value) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args(None, None).and_return(None) + 'parse_extra_options' + ).and_return((), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args(None, None, None).and_return(()) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('mysql', '--batch'), @@ -665,11 +701,14 @@ def test_restore_data_source_dump_runs_mysql_with_options(): 'resolve_credential' ).replace_with(lambda value, config: value) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args(None, None).and_return(None) + 'parse_extra_options' + ).and_return(('--harder',), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args(None, None, None).and_return(()) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( - ('mysql', '--batch', '--harder'), + ('mysql', '--harder', '--batch'), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, @@ -700,11 +739,14 @@ def test_restore_data_source_dump_runs_non_default_mysql_with_options(): 'resolve_credential' ).replace_with(lambda value, config: value) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args(None, None).and_return(None) + 'parse_extra_options' + ).and_return(('--harder',), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args(None, None, None).and_return(()) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( - ('custom_mysql', '--batch', '--harder'), + ('custom_mysql', '--harder', '--batch'), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, @@ -735,8 +777,11 @@ def test_restore_data_source_dump_runs_mysql_with_hostname_and_port(): 'resolve_credential' ).replace_with(lambda value, config: value) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args(None, None).and_return(None) + 'parse_extra_options' + ).and_return((), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args(None, None, None).and_return(()) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( @@ -779,8 +824,11 @@ def test_restore_data_source_dump_runs_mysql_with_username_and_password(): 'resolve_credential' ).replace_with(lambda value, config: value) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args('root', 'trustsome1').and_return(99) + 'parse_extra_options' + ).and_return((), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('mysql', '--defaults-extra-file=/dev/fd/99', '--batch'), @@ -824,8 +872,13 @@ def test_restore_data_source_dump_with_connection_params_uses_connection_params_ 'resolve_credential' ).replace_with(lambda value, config: value) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args('cliusername', 'clipassword').and_return(99) + 'parse_extra_options' + ).and_return((), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args('cliusername', 'clipassword', None).and_return( + ('--defaults-extra-file=/dev/fd/99',) + ) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( @@ -881,8 +934,13 @@ def test_restore_data_source_dump_without_connection_params_uses_restore_params_ 'resolve_credential' ).replace_with(lambda value, config: value) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args('restoreuser', 'restorepass').and_return(99) + 'parse_extra_options' + ).and_return((), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args('restoreuser', 'restorepass', None).and_return( + ('--defaults-extra-file=/dev/fd/99',) + ) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( @@ -925,8 +983,11 @@ def test_restore_data_source_dump_with_dry_run_skips_restore(): 'resolve_credential' ).replace_with(lambda value, config: value) flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( - 'make_defaults_file_pipe' - ).with_args(None, None).and_return(None) + 'parse_extra_options' + ).and_return((), None) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_options' + ).with_args(None, None, None).and_return(()) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').never() From 06b065cb09fef01e14375bca8cfe34f4ecfae2b2 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 28 Feb 2025 18:28:09 -0800 Subject: [PATCH 8/9] Add missing test coverage (#1009). --- borgmatic/hooks/data_source/mariadb.py | 59 ----- tests/unit/hooks/data_source/test_mariadb.py | 246 +++++++++---------- 2 files changed, 121 insertions(+), 184 deletions(-) diff --git a/borgmatic/hooks/data_source/mariadb.py b/borgmatic/hooks/data_source/mariadb.py index 985cd5b2..fc879142 100644 --- a/borgmatic/hooks/data_source/mariadb.py +++ b/borgmatic/hooks/data_source/mariadb.py @@ -143,65 +143,6 @@ def database_names_to_dump(database, config, username, password, environment, dr SYSTEM_DATABASE_NAMES = ('information_schema', 'mysql', 'performance_schema', 'sys') -def execute_dump_command( - database, - config, - username, - password, - dump_path, - database_names, - environment, - dry_run, - dry_run_label, -): - ''' - Kick off a dump for the given MariaDB database (provided as a configuration dict) to a named - pipe constructed from the given dump path and database name. - - Return a subprocess.Popen instance for the dump process ready to spew to a named pipe. But if - this is a dry run, then don't actually dump anything and return None. - ''' - database_name = database['name'] - dump_filename = dump.make_data_source_dump_filename( - dump_path, - database['name'], - database.get('hostname'), - database.get('port'), - ) - - if os.path.exists(dump_filename): - logger.warning( - f'Skipping duplicate dump of MariaDB database "{database_name}" to {dump_filename}' - ) - return None - - mariadb_dump_command = tuple( - shlex.quote(part) - for part in shlex.split(database.get('mariadb_dump_command') or 'mariadb-dump') - ) - extra_options, defaults_extra_filename = parse_extra_options(database.get('options')) - dump_command = ( - mariadb_dump_command - + make_defaults_file_options(username, password, defaults_extra_filename) - + extra_options - + (('--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 ()) - + ('--skip-column-names', '--batch') - + ('--execute', 'show schemas') - ) - - logger.debug('Querying for "all" MariaDB databases to dump') - - show_output = execute_command_and_capture_output(show_command, environment=environment) - - return tuple( - show_name - for show_name in show_output.strip().splitlines() - if show_name not in SYSTEM_DATABASE_NAMES - ) - - def execute_dump_command( database, config, diff --git a/tests/unit/hooks/data_source/test_mariadb.py b/tests/unit/hooks/data_source/test_mariadb.py index d425b82c..fe183865 100644 --- a/tests/unit/hooks/data_source/test_mariadb.py +++ b/tests/unit/hooks/data_source/test_mariadb.py @@ -6,10 +6,28 @@ from flexmock import flexmock from borgmatic.hooks.data_source import mariadb as module +def test_parse_extra_options_passes_through_empty_options(): + assert module.parse_extra_options('') == ((), None) + + +def test_parse_extra_options_with_defaults_extra_file_removes_and_and_parses_out_filename(): + assert module.parse_extra_options('--defaults-extra-file=extra.cnf --skip-ssl') == ( + ('--skip-ssl',), + 'extra.cnf', + ) + + +def test_parse_extra_options_without_defaults_extra_file_passes_through_options(): + assert module.parse_extra_options('--skip-ssl --and=stuff') == ( + ('--skip-ssl', '--and=stuff'), + None, + ) + + def test_make_defaults_file_pipe_without_username_or_password_bails(): flexmock(module.os).should_receive('pipe').never() - assert module.make_defaults_file_options(username=None, password=None) is () + assert module.make_defaults_file_options(username=None, password=None) == () def test_make_defaults_file_option_with_username_and_password_writes_them_to_file_descriptor(): @@ -23,10 +41,12 @@ def test_make_defaults_file_option_with_username_and_password_writes_them_to_fil flexmock(module.os).should_receive('close') flexmock(module.os).should_receive('set_inheritable') - assert module.make_defaults_file_options(username='root', password='trustsome1') == ('--defaults-extra-file=/dev/fd/99',) + assert module.make_defaults_file_options(username='root', password='trustsome1') == ( + '--defaults-extra-file=/dev/fd/99', + ) -def test_make_defaults_file_pipe_with_username_only_writes_it_to_file_descriptor(): +def test_make_defaults_file_pipe_with_only_username_writes_it_to_file_descriptor(): read_descriptor = 99 write_descriptor = flexmock() @@ -37,10 +57,12 @@ def test_make_defaults_file_pipe_with_username_only_writes_it_to_file_descriptor flexmock(module.os).should_receive('close') flexmock(module.os).should_receive('set_inheritable') - assert module.make_defaults_file_options(username='root', password=None) == ('--defaults-extra-file=/dev/fd/99',) + assert module.make_defaults_file_options(username='root', password=None) == ( + '--defaults-extra-file=/dev/fd/99', + ) -def test_make_defaults_file_pipe_with_password_only_writes_it_to_file_descriptor(): +def test_make_defaults_file_pipe_with_only_password_writes_it_to_file_descriptor(): read_descriptor = 99 write_descriptor = flexmock() @@ -51,7 +73,9 @@ def test_make_defaults_file_pipe_with_password_only_writes_it_to_file_descriptor flexmock(module.os).should_receive('close') flexmock(module.os).should_receive('set_inheritable') - assert module.make_defaults_file_options(username=None, password='trustsome1') == ('--defaults-extra-file=/dev/fd/99',) + assert module.make_defaults_file_options(username=None, password='trustsome1') == ( + '--defaults-extra-file=/dev/fd/99', + ) def test_make_defaults_file_option_with_defaults_extra_filename_includes_it_in_file_descriptor(): @@ -65,7 +89,17 @@ def test_make_defaults_file_option_with_defaults_extra_filename_includes_it_in_f flexmock(module.os).should_receive('close') flexmock(module.os).should_receive('set_inheritable') - assert module.make_defaults_file_options(username='root', password='trustsome1', defaults_extra_filename='extra.cnf') == ('--defaults-extra-file=/dev/fd/99',) + assert module.make_defaults_file_options( + username='root', password='trustsome1', defaults_extra_filename='extra.cnf' + ) == ('--defaults-extra-file=/dev/fd/99',) + + +def test_make_defaults_file_option_with_only_defaults_extra_filename_uses_it_instead_of_file_descriptor(): + flexmock(module.os).should_receive('pipe').never() + + assert module.make_defaults_file_options( + username=None, password=None, defaults_extra_filename='extra.cnf' + ) == ('--defaults-extra-file=extra.cnf',) def test_database_names_to_dump_passes_through_name(): @@ -94,12 +128,10 @@ def test_database_names_to_dump_queries_mariadb_for_database_names(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return((), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) + flexmock(module).should_receive('parse_extra_options').and_return((), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + 'root', 'trustsome1', None + ).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module).should_receive('execute_command_and_capture_output').with_args( ( 'mariadb', @@ -274,12 +306,12 @@ def test_dump_data_sources_dumps_all_databases_separately_when_format_configured def test_database_names_to_dump_runs_mariadb_with_list_options(): database = {'name': 'all', 'list_options': '--defaults-extra-file=mariadb.cnf --skip-ssl'} - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return(('--skip-ssl',), 'mariadb.cnf') - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args('root', 'trustsome1', 'mariadb.cnf').and_return(('--defaults-extra-file=/dev/fd/99',)) + flexmock(module).should_receive('parse_extra_options').and_return( + ('--skip-ssl',), 'mariadb.cnf' + ) + flexmock(module).should_receive('make_defaults_file_options').with_args( + 'root', 'trustsome1', 'mariadb.cnf' + ).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module).should_receive('execute_command_and_capture_output').with_args( ( 'mariadb', @@ -305,12 +337,12 @@ def test_database_names_to_dump_runs_non_default_mariadb_with_list_options(): 'list_options': '--defaults-extra-file=mariadb.cnf --skip-ssl', 'mariadb_command': 'custom_mariadb', } - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return(('--skip-ssl',), 'mariadb.cnf') - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args('root', 'trustsome1', 'mariadb.cnf').and_return(('--defaults-extra-file=/dev/fd/99',)) + flexmock(module).should_receive('parse_extra_options').and_return( + ('--skip-ssl',), 'mariadb.cnf' + ) + flexmock(module).should_receive('make_defaults_file_options').with_args( + 'root', 'trustsome1', 'mariadb.cnf' + ).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module).should_receive('execute_command_and_capture_output').with_args( environment=None, full_command=( @@ -337,12 +369,10 @@ def test_execute_dump_command_runs_mariadb_dump(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return((), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) + flexmock(module).should_receive('parse_extra_options').and_return((), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + 'root', 'trustsome1', None + ).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -382,12 +412,10 @@ def test_execute_dump_command_runs_mariadb_dump_without_add_drop_database(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return((), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) + flexmock(module).should_receive('parse_extra_options').and_return((), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + 'root', 'trustsome1', None + ).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -426,12 +454,10 @@ def test_execute_dump_command_runs_mariadb_dump_with_hostname_and_port(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return((), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) + flexmock(module).should_receive('parse_extra_options').and_return((), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + 'root', 'trustsome1', None + ).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -477,12 +503,10 @@ def test_execute_dump_command_runs_mariadb_dump_with_username_and_password(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return((), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) + flexmock(module).should_receive('parse_extra_options').and_return((), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + 'root', 'trustsome1', None + ).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -522,12 +546,10 @@ def test_execute_dump_command_runs_mariadb_dump_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return(('--stuff=such',), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) + flexmock(module).should_receive('parse_extra_options').and_return(('--stuff=such',), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + 'root', 'trustsome1', None + ).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -568,12 +590,10 @@ def test_execute_dump_command_runs_non_default_mariadb_dump_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return(('--stuff=such',), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) + flexmock(module).should_receive('parse_extra_options').and_return(('--stuff=such',), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + 'root', 'trustsome1', None + ).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( @@ -614,12 +634,10 @@ def test_execute_dump_command_runs_non_default_mariadb_dump_with_options(): def test_execute_dump_command_with_duplicate_dump_skips_mariadb_dump(): flexmock(module.dump).should_receive('make_data_source_dump_filename').and_return('dump') flexmock(module.os.path).should_receive('exists').and_return(True) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return((), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) + flexmock(module).should_receive('parse_extra_options').and_return((), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + 'root', 'trustsome1', None + ).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump').never() flexmock(module).should_receive('execute_command').never() @@ -645,12 +663,10 @@ def test_execute_dump_command_with_dry_run_skips_mariadb_dump(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return((), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) + flexmock(module).should_receive('parse_extra_options').and_return((), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + 'root', 'trustsome1', None + ).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').never() @@ -726,12 +742,10 @@ def test_restore_data_source_dump_runs_mariadb_to_restore(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return((), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args(None, None, None).and_return(()) + flexmock(module).should_receive('parse_extra_options').and_return((), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + None, None, None + ).and_return(()) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('mariadb', '--batch'), @@ -764,12 +778,10 @@ def test_restore_data_source_dump_runs_mariadb_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return(('--harder',), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args(None, None, None).and_return(()) + flexmock(module).should_receive('parse_extra_options').and_return(('--harder',), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + None, None, None + ).and_return(()) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('mariadb', '--harder', '--batch'), @@ -804,12 +816,10 @@ def test_restore_data_source_dump_runs_non_default_mariadb_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return(('--harder',), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args(None, None, None).and_return(()) + flexmock(module).should_receive('parse_extra_options').and_return(('--harder',), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + None, None, None + ).and_return(()) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('custom_mariadb', '--harder', '--batch'), @@ -842,12 +852,10 @@ def test_restore_data_source_dump_runs_mariadb_with_hostname_and_port(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return((), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args(None, None, None).and_return(()) + flexmock(module).should_receive('parse_extra_options').and_return((), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + None, None, None + ).and_return(()) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( @@ -889,12 +897,10 @@ def test_restore_data_source_dump_runs_mariadb_with_username_and_password(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return((), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args('root', 'trustsome1', None).and_return(('--defaults-extra-file=/dev/fd/99',)) + flexmock(module).should_receive('parse_extra_options').and_return((), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + 'root', 'trustsome1', None + ).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ('mariadb', '--defaults-extra-file=/dev/fd/99', '--batch'), @@ -937,14 +943,10 @@ def test_restore_data_source_dump_with_connection_params_uses_connection_params_ flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return((), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args('cliusername', 'clipassword', None).and_return( - ('--defaults-extra-file=/dev/fd/99',) - ) + flexmock(module).should_receive('parse_extra_options').and_return((), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + 'cliusername', 'clipassword', None + ).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( @@ -999,14 +1001,10 @@ def test_restore_data_source_dump_without_connection_params_uses_restore_params_ flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return((), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args('restoreuser', 'restorepass', None).and_return( - ('--defaults-extra-file=/dev/fd/99',) - ) + flexmock(module).should_receive('parse_extra_options').and_return((), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + 'restoreuser', 'restorepass', None + ).and_return(('--defaults-extra-file=/dev/fd/99',)) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( @@ -1048,12 +1046,10 @@ def test_restore_data_source_dump_with_dry_run_skips_restore(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) - flexmock(module).should_receive( - 'parse_extra_options' - ).and_return((), None) - flexmock(module).should_receive( - 'make_defaults_file_options' - ).with_args(None, None, None).and_return(()) + flexmock(module).should_receive('parse_extra_options').and_return((), None) + flexmock(module).should_receive('make_defaults_file_options').with_args( + None, None, None + ).and_return(()) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').never() From 839862cff05773c917fab20b46db51a8e8833129 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 28 Feb 2025 19:31:22 -0800 Subject: [PATCH 9/9] Update documentation link text about providing database passwords from external sources (#1009). --- docs/how-to/backup-your-databases.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/docs/how-to/backup-your-databases.md b/docs/how-to/backup-your-databases.md index 769d4dbf..30a3590f 100644 --- a/docs/how-to/backup-your-databases.md +++ b/docs/how-to/backup-your-databases.md @@ -309,10 +309,8 @@ hooks: ### External passwords If you don't want to keep your database passwords in your borgmatic -configuration file, you can instead pass them in via [environment -variables](https://torsion.org/borgmatic/docs/how-to/provide-your-passwords/) -or command-line [configuration -overrides](https://torsion.org/borgmatic/docs/how-to/make-per-application-backups/#configuration-overrides). +configuration file, you can instead pass them in [from external credential +sources](https://torsion.org/borgmatic/docs/how-to/provide-your-passwords/). ### Configuration backups