Skip to content

fix: make db migrations idempotent #981

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kyteinsky merged 3 commits into from
Aug 24, 2026
Merged

fix: make db migrations idempotent #981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b6db40c
fix: make db migrations idempotent and error tolerant
kyteinsky Aug 20, 2026
3f85de3
fix: do not re-encrypt already-encrypted secrets when a migration is …
oleksandr-nc Aug 21, 2026
b622feb
fix: make more db migrations idempotent
kyteinsky Aug 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions lib/Migration/Version032002Date20250527174907.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;
use OCP\Security\ICrypto;
use Throwable;

#[AddColumn('preferences_ex', 'sensitive', ColumnType::SMALLINT, 'support sensitive setting')]
class Version032002Date20250527174907 extends SimpleMigrationStep {
Expand Down Expand Up @@ -71,6 +72,11 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array
while ($row = $req->fetch()) {
$configValue = $row['configvalue'];
if (!empty($configValue)) {
// Readers decrypt exactly once, so re-encrypting an already-encrypted value would
// nest a second envelope and make the stored secret unrecoverable on the next read.
if ($this->isEncrypted((string)$configValue)) {
continue;
}
try {
$encryptedValue = $this->crypto->encrypt($configValue);
$qbUpdate = $this->connection->getQueryBuilder();
Expand All @@ -89,4 +95,17 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array
$req->closeCursor();
return null;
}

/**
* ICrypto::decrypt() HMAC-verifies the envelope, so a value that decrypts cleanly is already
* encrypted and must not be encrypted again.
*/
private function isEncrypted(string $value): bool {
try {
$this->crypto->decrypt($value);
return true;
} catch (Throwable) {
return false;
}
}
}
34 changes: 22 additions & 12 deletions lib/Migration/Version2000Date20240120094952.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,30 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

$table = $schema->getTable('ex_apps');
if ($table->hasColumn('protocol')) {
$table->dropColumn('protocol');
if ($schema->hasTable('ex_apps')) {
$table = $schema->getTable('ex_apps');
if ($table->hasColumn('protocol')) {
$table->dropColumn('protocol');
}
if ($table->hasColumn('host')) {
$table->dropColumn('host');
}
// would result in re-creation of the same index if this migration is re-run
// but should be safe to run.
if ($table->hasIndex('ex_apps_c_port__idx')) {
$table->dropIndex('ex_apps_c_port__idx');
}
$table->addUniqueIndex(['daemon_config_name', 'port'], 'ex_apps_c_port__idx');
}
if ($table->hasColumn('host')) {
$table->dropColumn('host');
}
$table->dropIndex('ex_apps_c_port__idx');
$table->addUniqueIndex(['daemon_config_name', 'port'], 'ex_apps_c_port__idx');

$table = $schema->getTable('ex_apps_daemons');
$table->changeColumn('deploy_config', [
'notnull' => true,
]);
if ($schema->hasTable('ex_apps_daemons')) {
$table = $schema->getTable('ex_apps_daemons');
if ($table->hasColumn('deploy_config')) {
$table->changeColumn('deploy_config', [
'notnull' => true,
]);
}
}
Comment thread
kyteinsky marked this conversation as resolved.

return $schema;
}
Expand Down
20 changes: 12 additions & 8 deletions lib/Migration/Version2201Date20240221124152.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,18 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

$table = $schema->getTable('ex_apps');

$table->addColumn('is_system', Types::SMALLINT, [
'notnull' => true,
'default' => 0,
'length' => 1,
'unsigned' => true,
]);
if ($schema->hasTable('ex_apps')) {
$table = $schema->getTable('ex_apps');

if (!$table->hasColumn('is_system')) {
$table->addColumn('is_system', Types::SMALLINT, [
'notnull' => true,
'default' => 0,
'length' => 1,
'unsigned' => true,
]);
}
}

return $schema;
}
Expand Down
14 changes: 9 additions & 5 deletions lib/Migration/Version2203Date20240325124149.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,15 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

$table = $schema->getTable('ex_apps');

$table->addColumn('api_scopes', Types::JSON, [
'notnull' => false,
]);
if ($schema->hasTable('ex_apps')) {
$table = $schema->getTable('ex_apps');

if (!$table->hasColumn('api_scopes')) {
$table->addColumn('api_scopes', Types::JSON, [
'notnull' => false,
]);
}
}

return $schema;
}
Expand Down
12 changes: 7 additions & 5 deletions lib/Migration/Version2206Date20240502145029.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,13 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
if ($schema->hasTable('ex_ui_files_actions')) {
$table = $schema->getTable('ex_ui_files_actions');

$table->addColumn('version', Types::STRING, [
'notnull' => true,
'length' => 64,
'default' => '1.0',
]);
if (!$table->hasColumn('version')) {
$table->addColumn('version', Types::STRING, [
'notnull' => true,
'length' => 64,
'default' => '1.0',
]);
}
}

return $schema;
Expand Down
13 changes: 8 additions & 5 deletions lib/Migration/Version2800Date20240711080316.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,14 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

$table = $schema->getTable('ex_task_processing');
if (!$table->hasColumn('custom_task_type')) {
$table->addColumn('custom_task_type', Types::TEXT, [
'notnull' => false,
]);
if ($schema->hasTable('ex_task_processing')) {
$table = $schema->getTable('ex_task_processing');

if (!$table->hasColumn('custom_task_type')) {
$table->addColumn('custom_task_type', Types::TEXT, [
'notnull' => false,
]);
}
}

return $schema;
Expand Down
13 changes: 8 additions & 5 deletions lib/Migration/Version3000Date20240807085759.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,14 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

$table = $schema->getTable('ex_task_processing');
if (!$table->hasColumn('provider')) {
$table->addColumn('provider', Types::TEXT, [
'notnull' => true,
]);
if ($schema->hasTable('ex_task_processing')) {
$table = $schema->getTable('ex_task_processing');

if (!$table->hasColumn('provider')) {
$table->addColumn('provider', Types::TEXT, [
'notnull' => true,
]);
}
}

return $schema;
Expand Down
15 changes: 9 additions & 6 deletions lib/Migration/Version3100Date20240822080316.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,15 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

$table = $schema->getTable('ex_apps_routes');
if (!$table->hasColumn('bruteforce_protection')) {
$table->addColumn('bruteforce_protection', Types::STRING, [
'notnull' => false,
'length' => 512,
]);
if ($schema->hasTable('ex_apps_routes')) {
$table = $schema->getTable('ex_apps_routes');

if (!$table->hasColumn('bruteforce_protection')) {
$table->addColumn('bruteforce_protection', Types::STRING, [
'notnull' => false,
'length' => 512,
]);
}
}

return $schema;
Expand Down
15 changes: 9 additions & 6 deletions lib/Migration/Version3200Date20240905080316.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,15 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

$table = $schema->getTable('ex_apps');
if ($table->hasColumn('last_check_time')) {
$table->dropColumn('last_check_time');
}
if ($table->hasColumn('api_scopes')) {
$table->dropColumn('api_scopes');
if ($schema->hasTable('ex_apps')) {
$table = $schema->getTable('ex_apps');

if ($table->hasColumn('last_check_time')) {
$table->dropColumn('last_check_time');
}
if ($table->hasColumn('api_scopes')) {
$table->dropColumn('api_scopes');
}
}

return $schema;
Expand Down
20 changes: 20 additions & 0 deletions lib/Migration/Version5000Date20241120135411.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;
use OCP\Security\ICrypto;
use Throwable;

class Version5000Date20241120135411 extends SimpleMigrationStep {

Expand Down Expand Up @@ -42,6 +43,12 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array
$deployConfig = $row['deploy_config'];
$deployConfig = json_decode($deployConfig, true);
if (!empty($deployConfig['haproxy_password'])) {
// Every consumer decrypts exactly once, so a value that is already ciphertext must be
// left alone: wrapping a second envelope around it silently turns the daemon key into
// garbage. This step is replayed whenever the migration was applied but not recorded.
if ($this->isEncrypted((string)$deployConfig['haproxy_password'])) {
continue;
}
$deployConfig['haproxy_password'] = $this->crypto->encrypt($deployConfig['haproxy_password']);
$encodedDeployConfig = json_encode($deployConfig);
$qbUpdate = $this->connection->getQueryBuilder();
Expand All @@ -56,4 +63,17 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array
$req->closeCursor();
return null;
}

/**
* ICrypto::decrypt() HMAC-verifies the envelope, so a value that decrypts cleanly is already
* encrypted and must not be encrypted again.
*/
private function isEncrypted(string $value): bool {
try {
$this->crypto->decrypt($value);
return true;
} catch (Throwable) {
return false;
}
}
}
Loading