mirror/snipe-it
3
0
Fork 0
mirror of https://github.com/snipe/snipe-it.git synced 2026-08-18 03:06:23 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #19507 from grokability/db-restore-fixes-for-mariadb

Browse Source 
Backup/Restore: Fixed #19506 and  #19506 - MariaDB executable/path change
This commit is contained in:
snipe
2026-08-17 22:35:25 +01:00
committed by GitHub
parent 13fb3fde01 8b899eaaff
commit 95fb74351a
5 changed files with 216 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
.env.docker
View File

@ -26,7 +26,9 @@ PUBLIC_FILESYSTEM_DISK=local_public
# --------------------------------------------
# REQUIRED: DATABASE SETTINGS
# --------------------------------------------
DB_CONNECTION=mysql
# The docker-compose db service uses MariaDB, so we route through the mariadb
# driver so spatie's backup uses mariadb-dump + --skip-ssl.
DB_CONNECTION=mariadb
DB_HOST=db
DB_SOCKET=null
DB_PORT='3306'
@ -36,7 +38,10 @@ DB_PASSWORD=changeme1234
MYSQL_ROOT_PASSWORD=changeme1234
DB_PREFIX=null
DB_DUMP_PATH='/usr/bin'
DB_DUMP_SKIP_SSL=true
# Left off by default: the composed mariadb service has no TLS configured,
# so mariadb-dump connects unencrypted regardless. Opt in only if your
# server actually requires an SSL-skip flag.
DB_DUMP_SKIP_SSL=false
DB_DUMP_SINGLE_TRANSACTION=false
DB_CHARSET=utf8mb4
DB_COLLATION=utf8mb4_unicode_ci

3
.env.example
View File

@ -22,6 +22,9 @@ PUBLIC_FILESYSTEM_DISK=local_public
# --------------------------------------------
# REQUIRED: DATABASE SETTINGS
# --------------------------------------------
# Set DB_CONNECTION=mariadb if your server is MariaDB. This makes the
# backup/restore tooling invoke mariadb / mariadb-dump instead of the
# deprecated mysql / mysqldump symlinks.
DB_CONNECTION=mysql
DB_HOST=127.0.0.1
DB_SOCKET=null

80
app/Console/Commands/RestoreFromBackup.php
View File

@ -202,6 +202,28 @@ class RestoreFromBackup extends Command
parent::__construct();
}
/**
* Pick the DB client binary to invoke for a given driver. Prefers the
* driver-native name (mariadb for the mariadb driver, mysql otherwise),
* falls back to the other name if the preferred binary is missing.
* Returns null if neither exists in the given path.
*/
public static function pickDbClientBinary(string $driver, string $binaryPath): ?string
{
$ext = \DIRECTORY_SEPARATOR === '\\' ? '.exe' : '';
$preferred = $driver === 'mariadb' ? 'mariadb' : 'mysql';
$fallback = $preferred === 'mariadb' ? 'mysql' : 'mariadb';
foreach ([$preferred, $fallback] as $name) {
$candidate = rtrim($binaryPath, \DIRECTORY_SEPARATOR) . \DIRECTORY_SEPARATOR . $name . $ext;
if (file_exists($candidate)) {
return $candidate;
}
}
return null;
}
/**
* Execute the console command.
*
@ -225,8 +247,12 @@ class RestoreFromBackup extends Command
return $this->error('Data loss not confirmed');
}
if (config('database.default') != 'mysql') {
return $this->error('DB_CONNECTION must be MySQL in order to perform a restore. Detected: '.config('database.default'));
$connectionName = config('database.default');
$connectionConfig = config("database.connections.$connectionName");
$driver = $connectionConfig['driver'] ?? null;
if (!in_array($driver, ['mysql', 'mariadb'], true)) {
return $this->error('DB_CONNECTION must be MySQL or MariaDB in order to perform a restore. Detected driver: ' . ($driver ?? 'unknown') . " (connection: $connectionName)");
}
$za = new ZipArchive;
@ -430,7 +456,8 @@ class RestoreFromBackup extends Command
$sql_contents = $za->getStream($sql_stat['name']); // maybe copy *THIS* thing?
if ($sql_contents === false) {
$this->error("Unable to open SQL file: " . $sql_stat['name']);
$this->error('Unable to open SQL file: ' . $sql_stat['name']);
return -1;
}
@ -456,26 +483,29 @@ class RestoreFromBackup extends Command
$pipes = [];
$env_vars = getenv();
$env_vars['MYSQL_PWD'] = config('database.connections.mysql.password');
// TODO notes: we are stealing the dump_binary_path (which *probably* also has your copy of the mysql binary in it. But it might not, so we might need to extend this)
// we unilaterally prepend a slash to the `mysql` command. This might mean your path could look like /blah/blah/blah//mysql - which should be fine. But maybe in some environments it isn't?
$mysql_binary = config('database.connections.mysql.dump.dump_binary_path').\DIRECTORY_SEPARATOR.'mysql'.(\DIRECTORY_SEPARATOR == '\\' ? '.exe' : '');
if (! file_exists($mysql_binary)) {
return $this->error("mysql tool at: '$mysql_binary' does not exist, cannot restore. Please edit DB_DUMP_PATH in your .env to point to a directory that contains the mysqldump and mysql binary");
$env_vars['MYSQL_PWD'] = $connectionConfig['password'];
$binaryPath = $connectionConfig['dump']['dump_binary_path'] ?? '';
$client_binary = static::pickDbClientBinary($driver, $binaryPath);
if ($client_binary === null) {
$preferred = $driver === 'mariadb' ? 'mariadb' : 'mysql';
return $this->error("DB client binary '$preferred' not found in DB_DUMP_PATH ('$binaryPath'). Please edit DB_DUMP_PATH in your .env to point to a directory that contains the mysql/mariadb client binary.");
}
$proc_results = proc_open("$mysql_binary -h " .
escapeshellarg(config('database.connections.mysql.host')) .
$proc_results = proc_open(escapeshellarg($client_binary) . ' -h ' .
escapeshellarg($connectionConfig['host']) .
' --batch ' .
' --binary-mode ' .
' -u ' . escapeshellarg(config('database.connections.mysql.username')) . ' ' .
' -P ' . escapeshellarg(config('database.connections.mysql.port')) . ' ' .
escapeshellarg(config('database.connections.mysql.database')), // yanked -p since we pass via ENV
' -u ' . escapeshellarg($connectionConfig['username']) . ' ' .
' -P ' . escapeshellarg($connectionConfig['port']) . ' ' .
escapeshellarg($connectionConfig['database']), // yanked -p since we pass via ENV
[0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']],
$pipes,
null,
$env_vars); // this is not super-duper awesome-secure, but definitely more secure than showing it on the CLI, or dropping temporary files with passwords in them.
if ($proc_results === false) {
return $this->error('Unable to invoke mysql via CLI');
return $this->error('Unable to invoke DB client via CLI');
}
try {
@ -497,17 +527,31 @@ class RestoreFromBackup extends Command
}
} catch (\Exception $e) {
Log::error('Error during restore!!!! '.$e->getMessage());
// FIXME - put these back and/or put them in the right places?!
$err_out = fgets($pipes[1]);
$err_err = fgets($pipes[2]);
// Drain both pipes fully so the DB client's own diagnostic ends up in
// the log instead of just the downstream "broken pipe". Then include
// the process exit code so root-cause is visible without strace.
// The deprecation warning about maria-db is a red herring, and not the actual problem.
// "Deprecated program name. It will be removed in a future release, use '/usr/bin/mariadb' instead"
// was drowning out the actual problem.
stream_set_blocking($pipes[1], false);
stream_set_blocking($pipes[2], false);
$err_out = stream_get_contents($pipes[1]) ?: '';
$err_err = stream_get_contents($pipes[2]) ?: '';
fclose($pipes[0]);
fclose($pipes[1]);
fclose($pipes[2]);
$exit_code = proc_close($proc_results);
Log::error('Error OUTPUT: '.$err_out);
$this->info($err_out);
Log::error('Error ERROR : '.$err_err);
$this->error($err_err);
Log::error("DB client exited with code $exit_code");
$this->error("DB client exited with code $exit_code");
throw $e;
}
if (! feof($sql_contents) || $bytes_read == 0) {
$this->error('Not at end of file for sql file, or zero bytes read. aborting!');
return -1;
}

46
config/database.php
View File

@ -21,10 +21,16 @@ $dump_options = [
// that require --single-transaction (e.g. no LOCK TABLES privilege).
$dump_options['use_single_transaction'] = (env('DB_DUMP_SINGLE_TRANSACTION', 'false') === 'true');
// For modern versions of mysqldump, use --ssl-mode=DISABLED
if (env('DB_DUMP_SKIP_SSL') == 'true') {
// Correctly add the option as a string to the 'add_extra_option' key.
$dump_options['add_extra_option'] = '--ssl-mode=DISABLED';
// Opt-in flag for skipping SSL on the dump connection. Only add the key when
// it's true - spatie's processExtraDumpParameters has a bug where a `false`
// value routes through callMethodOnDumper's `if (!$methodValue)` branch, which
// calls setSkipSsl() with no args, and that setter defaults to true. So an
// explicit false would silently become true. When opted in, spatie emits the
// correct per-DBMS flag: --ssl-mode=DISABLED for MySQL 8.4+, --skip-ssl for
// older MySQL and MariaDB. Do NOT set this via add_extra_option; that route
// ships the raw flag as-is and mariadb-dump rejects ssl-mode.
if (env('DB_DUMP_SKIP_SSL') === 'true') {
$dump_options['skip_ssl'] = true;
}
return [
@ -113,6 +119,38 @@ return [
]) : [],
],
// Use this connection (DB_CONNECTION=mariadb) when the server is
// MariaDB. Spatie's backup picks the MariaDb dumper for this driver,
// which invokes mariadb-dump instead of the deprecated mysqldump
// symlink and uses --skip-ssl instead of --ssl-mode=DISABLED.
'mariadb' => [
'driver' => 'mariadb',
'host' => env('DB_HOST', 'localhost'),
'port' => (int) env('DB_PORT', 3306),
'database' => env('DB_DATABASE', 'forge'),
'username' => env('DB_USERNAME', 'forge'),
'password' => env('DB_PASSWORD', ''),
'charset' => env('DB_CHARSET', 'utf8mb4'),
'collation' => env('DB_COLLATION', 'utf8mb4_unicode_ci'),
'prefix' => env('DB_PREFIX', null),
'strict' => false,
'engine' => 'InnoDB',
'unix_socket' => env('DB_SOCKET', ''),
'dump' => $dump_options,
'dump_command_timeout' => 60 * 5,
'dump_using_single_transaction' => true,
'options' => (env('DB_SSL')) ? ((env('DB_SSL_IS_PAAS')) ? [
PDO::MYSQL_ATTR_SSL_CA => env('DB_SSL_CA_PATH'),
PDO::MYSQL_ATTR_SSL_VERIFY_SERVER_CERT => env('DB_SSL_VERIFY_SERVER', false),
] : [
PDO::MYSQL_ATTR_SSL_KEY => env('DB_SSL_KEY_PATH'),
PDO::MYSQL_ATTR_SSL_CERT => env('DB_SSL_CERT_PATH'),
PDO::MYSQL_ATTR_SSL_CA => env('DB_SSL_CA_PATH'),
PDO::MYSQL_ATTR_SSL_CIPHER => env('DB_SSL_CIPHER'),
PDO::MYSQL_ATTR_SSL_VERIFY_SERVER_CERT => env('DB_SSL_VERIFY_SERVER', false),
]) : [],
],
'pgsql' => [
'driver' => 'pgsql',
'host' => env('DB_HOST', 'localhost'),

102
tests/Feature/Console/RestoreFromBackupBinaryPickerTest.php Normal file
View File

@ -0,0 +1,102 @@
<?php
namespace Tests\Feature\Console;
use App\Console\Commands\RestoreFromBackup;
use Tests\TestCase;
class RestoreFromBackupBinaryPickerTest extends TestCase
{
private string $tmpDir;
protected function setUp(): void
{
parent::setUp();
$this->tmpDir = sys_get_temp_dir().'/snipeit-restore-picker-'.uniqid();
mkdir($this->tmpDir, 0755, true);
}
protected function tearDown(): void
{
array_map('unlink', glob($this->tmpDir.'/*') ?: []);
rmdir($this->tmpDir);
parent::tearDown();
}
private function makeBinary(string $name): void
{
touch($this->tmpDir.\DIRECTORY_SEPARATOR.$name.(\DIRECTORY_SEPARATOR === '\\' ? '.exe' : ''));
}
public function test_mariadb_driver_prefers_mariadb_binary(): void
{
$this->makeBinary('mysql');
$this->makeBinary('mariadb');
$picked = RestoreFromBackup::pickDbClientBinary('mariadb', $this->tmpDir);
$this->assertSame($this->tmpDir.\DIRECTORY_SEPARATOR.'mariadb'.(\DIRECTORY_SEPARATOR === '\\' ? '.exe' : ''), $picked);
}
public function test_mysql_driver_prefers_mysql_binary(): void
{
$this->makeBinary('mysql');
$this->makeBinary('mariadb');
$picked = RestoreFromBackup::pickDbClientBinary('mysql', $this->tmpDir);
$this->assertSame($this->tmpDir.\DIRECTORY_SEPARATOR.'mysql'.(\DIRECTORY_SEPARATOR === '\\' ? '.exe' : ''), $picked);
}
public function test_mariadb_driver_falls_back_to_mysql_binary_when_mariadb_missing(): void
{
$this->makeBinary('mysql');
$picked = RestoreFromBackup::pickDbClientBinary('mariadb', $this->tmpDir);
$this->assertSame($this->tmpDir.\DIRECTORY_SEPARATOR.'mysql'.(\DIRECTORY_SEPARATOR === '\\' ? '.exe' : ''), $picked);
}
public function test_mysql_driver_falls_back_to_mariadb_binary_when_mysql_missing(): void
{
$this->makeBinary('mariadb');
$picked = RestoreFromBackup::pickDbClientBinary('mysql', $this->tmpDir);
$this->assertSame($this->tmpDir.\DIRECTORY_SEPARATOR.'mariadb'.(\DIRECTORY_SEPARATOR === '\\' ? '.exe' : ''), $picked);
}
public function test_returns_null_when_no_binary_exists(): void
{
$picked = RestoreFromBackup::pickDbClientBinary('mysql', $this->tmpDir);
$this->assertNull($picked);
}
public function test_trailing_separator_on_path_does_not_double(): void
{
$this->makeBinary('mariadb');
$picked = RestoreFromBackup::pickDbClientBinary('mariadb', $this->tmpDir.\DIRECTORY_SEPARATOR);
$this->assertSame($this->tmpDir.\DIRECTORY_SEPARATOR.'mariadb'.(\DIRECTORY_SEPARATOR === '\\' ? '.exe' : ''), $picked);
}
public function test_config_exposes_mariadb_connection_with_correct_driver(): void
{
$this->assertSame('mariadb', config('database.connections.mariadb.driver'));
}
public function test_skip_ssl_is_absent_from_dump_config_by_default(): void
{
// Regression: spatie's processExtraDumpParameters iterates the dump
// config and treats a `false` value as "call the setter with no args",
// which for setSkipSsl(bool $x = true) silently sets skipSsl to TRUE.
// The key must be absent from the array (not set to false) when opted
// out, matching the old add_extra_option gating shape.
$this->assertArrayNotHasKey('skip_ssl', config('database.connections.mysql.dump'));
$this->assertArrayNotHasKey('skip_ssl', config('database.connections.mariadb.dump'));
$this->assertArrayNotHasKey('add_extra_option', config('database.connections.mysql.dump'));
$this->assertArrayNotHasKey('add_extra_option', config('database.connections.mariadb.dump'));
}
}