From 21bde13274bf49a96b58b1c780b6dc94ca8baa60 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 21 Jun 2023 17:24:32 +0200 Subject: [PATCH 01/48] Introduce database models required by migration hooks --- library/Icinga/Model/Schema.php | 49 ++++++++++++++++++++++++++++++++ phpstan.neon | 3 ++ schema/mysql-upgrades/2.12.0.sql | 11 +++++++ schema/mysql.schema.sql | 15 ++++++---- schema/pgsql-upgrades/2.12.0.sql | 13 +++++++++ schema/pgsql.schema.sql | 15 ++++++---- 6 files changed, 95 insertions(+), 11 deletions(-) create mode 100644 library/Icinga/Model/Schema.php create mode 100644 schema/mysql-upgrades/2.12.0.sql create mode 100644 schema/pgsql-upgrades/2.12.0.sql diff --git a/library/Icinga/Model/Schema.php b/library/Icinga/Model/Schema.php new file mode 100644 index 000000000..465cce084 --- /dev/null +++ b/library/Icinga/Model/Schema.php @@ -0,0 +1,49 @@ +add(new BoolCast(['success'])); + $behaviors->add(new MillisecondTimestamp(['timestamp'])); + } +} diff --git a/phpstan.neon b/phpstan.neon index 43cecf9c4..9da27bcff 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -49,6 +49,8 @@ parameters: count: 2 path: library/Icinga/Protocol/Ldap/LdapConnection.php + - '#Call to an undefined method ipl\\Sql\\Connection::exec\(\)#' + scanDirectories: - vendor @@ -56,6 +58,7 @@ parameters: - library/Icinga/Test universalObjectCratesClasses: + - ipl\Orm\Model - Icinga\Data\ConfigObject - Icinga\Web\View - Icinga\Module\Monitoring\Object\MonitoredObject diff --git a/schema/mysql-upgrades/2.12.0.sql b/schema/mysql-upgrades/2.12.0.sql new file mode 100644 index 000000000..9aace2856 --- /dev/null +++ b/schema/mysql-upgrades/2.12.0.sql @@ -0,0 +1,11 @@ +ALTER TABLE icingaweb_schema + MODIFY COLUMN timestamp bigint unsigned NOT NULL, + MODIFY COLUMN version varchar(64) NOT NULL, + ADD COLUMN IF NOT EXISTS success enum ('n', 'y') DEFAULT NULL, + ADD COLUMN IF NOT EXISTS reason text DEFAULT NULL, + DROP CONSTRAINT IF EXISTS idx_icingaweb_schema_version, + ADD CONSTRAINT idx_icingaweb_schema_version UNIQUE (version); + +INSERT INTO icingaweb_schema (version, timestamp, success, reason) + VALUES('2.12.0', UNIX_TIMESTAMP() * 1000, 'y', NULL) + ON DUPLICATE KEY UPDATE timestamp = VALUES(timestamp), success = VALUES(success), reason = VALUES(reason); diff --git a/schema/mysql.schema.sql b/schema/mysql.schema.sql index 160061887..bb37f9b28 100644 --- a/schema/mysql.schema.sql +++ b/schema/mysql.schema.sql @@ -54,12 +54,15 @@ CREATE TABLE `icingaweb_rememberme`( ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin ROW_FORMAT=DYNAMIC; CREATE TABLE icingaweb_schema ( - id int unsigned NOT NULL AUTO_INCREMENT, - version smallint unsigned NOT NULL, - timestamp int unsigned NOT NULL, + id int unsigned NOT NULL AUTO_INCREMENT, + version varchar(64) NOT NULL, + timestamp bigint NOT NULL, + success enum ('n', 'y') DEFAULT NULL, + reason text DEFAULT NULL, - PRIMARY KEY (id) + PRIMARY KEY (id), + CONSTRAINT idx_icingaweb_schema_version UNIQUE (version) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin ROW_FORMAT=DYNAMIC; -INSERT INTO icingaweb_schema (version, timestamp) - VALUES (6, UNIX_TIMESTAMP()); +INSERT INTO icingaweb_schema (version, timestamp, success) + VALUES ('2.12.0', UNIX_TIMESTAMP() * 1000, 'y'); diff --git a/schema/pgsql-upgrades/2.12.0.sql b/schema/pgsql-upgrades/2.12.0.sql new file mode 100644 index 000000000..733d8f547 --- /dev/null +++ b/schema/pgsql-upgrades/2.12.0.sql @@ -0,0 +1,13 @@ +CREATE TYPE boolenum AS ENUM ('n', 'y'); + +ALTER TABLE icingaweb_schema + ALTER COLUMN timestamp TYPE bigint, + ALTER COLUMN version TYPE varchar(64), + ADD COLUMN success boolenum DEFAULT NULL, + ADD COLUMN reason text DEFAULT NULL, + DROP CONSTRAINT IF EXISTS idx_icingaweb_schema_version, + ADD CONSTRAINT idx_icingaweb_schema_version UNIQUE (version); + +INSERT INTO icingaweb_schema (version, timestamp, success, reason) + VALUES('2.12.0', EXTRACT(EPOCH FROM now()) * 1000, 'y', NULL) + ON CONFLICT ON CONSTRAINT idx_icingaweb_schema_version DO UPDATE SET timestamp = EXCLUDED.timestamp, success = EXCLUDED.success, reason = EXCLUDED.reason; diff --git a/schema/pgsql.schema.sql b/schema/pgsql.schema.sql index 8bf4ca07f..3a5413bbb 100644 --- a/schema/pgsql.schema.sql +++ b/schema/pgsql.schema.sql @@ -118,13 +118,18 @@ ALTER TABLE ONLY "icingaweb_rememberme" "id" ); +CREATE TYPE boolenum AS ENUM ('n', 'y'); + CREATE TABLE "icingaweb_schema" ( "id" serial, - "version" smallint NOT NULL, - "timestamp" int NOT NULL, + "version" varchar(64) NOT NULL, + "timestamp" bigint NOT NULL, + "success" boolenum DEFAULT NULL, + "reason" text DEFAULT NULL, - CONSTRAINT pk_icingaweb_schema PRIMARY KEY ("id") + CONSTRAINT pk_icingaweb_schema PRIMARY KEY ("id"), + CONSTRAINT idx_icingaweb_schema_version UNIQUE (version) ); -INSERT INTO icingaweb_schema (version, timestamp) - VALUES (6, extract(epoch from now())); +INSERT INTO icingaweb_schema (version, timestamp, success) + VALUES ('2.12.0', extract(epoch from now()) * 1000, 'y'); From babc59437f9a4cf6d565b5087e5e3a020733b382 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Mon, 24 Jul 2023 13:10:47 +0200 Subject: [PATCH 02/48] Introduce base `MigrationHook` class & helpers --- .../Application/Hook/Common/DbMigration.php | 151 +++++++ .../Icinga/Application/Hook/MigrationHook.php | 379 ++++++++++++++++++ 2 files changed, 530 insertions(+) create mode 100644 library/Icinga/Application/Hook/Common/DbMigration.php create mode 100644 library/Icinga/Application/Hook/MigrationHook.php diff --git a/library/Icinga/Application/Hook/Common/DbMigration.php b/library/Icinga/Application/Hook/Common/DbMigration.php new file mode 100644 index 000000000..14219be58 --- /dev/null +++ b/library/Icinga/Application/Hook/Common/DbMigration.php @@ -0,0 +1,151 @@ +scriptPath = $scriptPath; + + $this->setVersion($version); + } + + /** + * Get the sql script version the queries are loaded from + * + * @return string + */ + public function getVersion(): string + { + return $this->version; + } + + /** + * Set the sql script version the queries are loaded from + * + * @param string $version + * + * @return $this + */ + public function setVersion(string $version): self + { + $this->version = $version; + + return $this; + } + + /** + * Get upgrade script relative path name + * + * @return string + */ + public function getScriptPath(): string + { + return $this->scriptPath; + } + + /** + * Get the description of this database migration if any + * + * @return ?string + */ + public function getDescription(): ?string + { + return $this->description; + } + + /** + * Set the description of this database migration + * + * @param ?string $description + * + * @return DbMigration + */ + public function setDescription(?string $description): self + { + $this->description = $description; + + return $this; + } + + /** + * Get the last error message of this hook if any + * + * @return ?string + */ + public function getLastState(): ?string + { + return $this->lastState; + } + + /** + * Set the last error message + * + * @param ?string $message + * + * @return $this + */ + public function setLastState(?string $message): self + { + $this->lastState = $message; + + return $this; + } + + /** + * Perform the sql migration + * + * @param Connection $conn + * + * @return $this + * + * @throws RuntimeException Throws an error in case of any database errors or when there is nothing to migrate + */ + public function apply(Connection $conn): self + { + if (! $this->query) { + $statements = @file_get_contents($this->getScriptPath()); + if (! $statements) { + throw new RuntimeException(sprintf('Cannot load upgrade script %s', $this->getScriptPath())); + } + + if (preg_match('/\s*delimiter\s*(\S+)\s*$/im', $statements, $matches)) { + /** @var string $statements */ + $statements = preg_replace('/\s*delimiter\s*(\S+)\s*$/im', '', $statements); + /** @var string $statements */ + $statements = preg_replace('/' . preg_quote($matches[1], '/') . '$/m', ';', $statements); + } + + $this->query = $statements; + } + + if (empty($this->query)) { + throw new RuntimeException('Nothing to migrate'); + } + + $conn->exec($this->query); + + return $this; + } +} diff --git a/library/Icinga/Application/Hook/MigrationHook.php b/library/Icinga/Application/Hook/MigrationHook.php new file mode 100644 index 000000000..bbbc529cc --- /dev/null +++ b/library/Icinga/Application/Hook/MigrationHook.php @@ -0,0 +1,379 @@ + All pending database migrations of this hook */ + protected $migrations; + + /** @var ?string The current version of this hook */ + protected $version; + + /** + * Get whether the specified table exists in the given database + * + * @param Connection $conn + * @param string $table + * + * @return bool + */ + public static function tableExists(Connection $conn, string $table): bool + { + /** @var stdClass $query */ + $query = $conn->prepexec( + 'SELECT EXISTS(SELECT 1 FROM information_schema.tables WHERE table_name = ?) AS result', + $table + )->fetch(PDO::FETCH_OBJ); + + return $query->result; + } + + /** + * Get whether the specified column exists in the provided table + * + * @param Connection $conn + * @param string $table + * @param string $column + * + * @return ?string + */ + public static function getColumnType(Connection $conn, string $table, string $column): ?string + { + $pdoStmt = $conn->prepexec( + sprintf( + "SELECT %s AS column_type, %s AS column_length FROM information_schema.columns WHERE table_name = ? AND column_name = ?", + $conn->getAdapter() instanceof Pgsql ? 'udt_name' : 'column_type', + $conn->getAdapter() instanceof Pgsql ? 'character_maximum_length' : 'NULL' + ), + [$table, $column] + ); + + /** @var false|stdClass $result */ + $result = $pdoStmt->fetch(PDO::FETCH_OBJ); + if ($result === false) { + return null; + } + + if ($result->column_length !== null) { + $result->column_type .= '(' . $result->column_length . ')'; + } + + return $result->column_type; + } + + /** + * Get statically provided descriptions of the individual migrate scripts + * + * @return string[] + */ + abstract public function providedDescriptions(): array; + + /** + * Get the full name of the component this hook is implemented by + * + * @return string + */ + abstract public function getName(): string; + + /** + * Get the current schema version of this migration hook + * + * @return string + */ + abstract public function getVersion(): string; + + /** + * Get all the pending migrations of this hook + * + * @return DbMigration[] + */ + public function getMigrations(): array + { + if ($this->migrations === null) { + $this->migrations = []; + + $this->load(); + } + + return $this->migrations; + } + + /** + * Get the latest migrations limited by the given number + * + * @param int $limit + * + * @return DbMigration[] + */ + public function getLatestMigrations(int $limit): array + { + $migrations = $this->getMigrations(); + if ($limit > 0) { + $migrations = array_slice($migrations, -$limit, null, true); + } + + return array_reverse($migrations); + } + + /** + * Apply all pending migrations of this hook + * + * @return bool Whether the migration(s) have been successfully applied + */ + public function run(): bool + { + $conn = $this->getDb(); + foreach ($this->getMigrations() as $migration) { + try { + $migration->apply($conn); + + $this->version = $migration->getVersion(); + unset($this->migrations[$migration->getVersion()]); + + Logger::error( + "Applied %s pending migration version %s successfully", + $this->getName(), + $migration->getVersion() + ); + + $this->storeState($migration->getVersion(), null); + } catch (Exception $e) { + Logger::error( + "Failed to apply %s pending migration version %s \n%s", + $this->getName(), + $migration->getVersion(), + $e->getMessage() + ); + Logger::debug($e->getTraceAsString()); + + static::insertFailedEntry( + $conn, + $this->getSchemaQueryFor($migration->getVersion()), + $migration->getVersion(), + $e->getMessage() . PHP_EOL . $e->getTraceAsString() + ); + + return false; + } + } + + return true; + } + + /** + * Get whether this hook is implemented by a module + * + * @return bool + */ + public function isModule(): bool + { + return ClassLoader::classBelongsToModule(static::class); + } + + /** + * Get the name of the module this hook is implemented by + * + * @return string + */ + public function getModuleName(): string + { + if (! $this->isModule()) { + return static::DEFAULT_MODULE; + } + + return ClassLoader::extractModuleName(static::class); + } + + /** + * Get the number of pending migrations of this hook + * + * @return int + */ + public function count(): int + { + return count($this->getMigrations()); + } + + /** + * Get a database connection + * + * @return Connection + */ + abstract protected function getDb(): Connection; + + /** + * Get a schema version query filtered by the given $version + * + * @param string $version + * + * @return Query + */ + abstract protected function getSchemaQueryFor(string $version): Query; + + protected function load(): void + { + $upgradeDir = static::MYSQL_UPGRADE_DIR; + if ($this->getDb()->getAdapter() instanceof Pgsql) { + $upgradeDir = static::PGSQL_UPGRADE_DIR; + } + + if (! $this->isModule()) { + $path = Icinga::app()->getBaseDir(); + } else { + $path = Module::get($this->getModuleName())->getBaseDir(); + } + + $descriptions = $this->providedDescriptions(); + $version = $this->getVersion(); + /** @var SplFileInfo $file */ + foreach (new DirectoryIterator($path . DIRECTORY_SEPARATOR . $upgradeDir) as $file) { + if (preg_match('/^(?:r|v)?((?:\d+\.){0,2}\d+)(?:_([\w+]+))?\.sql$/', $file->getFilename(), $m)) { + if (version_compare($m[1], $version, '>')) { + $migration = new DbMigration($m[1], $file->getRealPath()); + if (isset($descriptions[$migration->getVersion()])) { + $migration->setDescription($descriptions[$migration->getVersion()]); + } elseif (isset($m[2])) { + $migration->setDescription(str_replace('_', ' ', $m[2])); + } + + $migration->setLastState($this->loadLastState($migration->getVersion())); + + $this->migrations[$m[1]] = $migration; + } + } + } + + // Sort all the migrations by their version numbers in ascending order. + uksort($this->migrations, 'version_compare'); + } + + /** + * Insert failed migration entry into the database or to the session + * + * @param Connection $conn + * @param Query $schemaQuery + * @param string $version + * @param string $reason + * + * @return $this + */ + protected function insertFailedEntry(Connection $conn, Query $schemaQuery, string $version, string $reason): self + { + if (! static::getColumnType($conn, $schemaQuery->getModel()->getTableName(), 'success')) { + $this->storeState($version, $reason); + } else { + /** @var Schema $schema */ + $schema = $schemaQuery->first(); + if ($schema) { + $conn->update($schema->getTableName(), [ + 'timestamp' => (new DateTime())->getTimestamp() * 1000.0, + 'success' => 'n', + 'reason' => $reason + ], ['id = ?' => $schema->id]); + } else { + $conn->insert($schemaQuery->getModel()->getTableName(), [ + 'version' => $version, + 'timestamp' => (new DateTime())->getTimestamp() * 1000.0, + 'success' => 'n', + 'reason' => $reason + ]); + } + } + + return $this; + } + + /** + * Store a failed state message in the session for the given version + * + * @param string $version + * @param ?string $reason + * + * @return $this + */ + protected function storeState(string $version, ?string $reason): self + { + $session = Session::getSession()->getNamespace('migrations'); + /** @var array $states */ + $states = $session->get($this->getModuleName(), []); + $states[$version] = $reason; + + $session->set($this->getModuleName(), $states); + + return $this; + } + + /** + * Load last failed state from database/session for the given version + * + * @param string $version + * + * @return ?string + */ + protected function loadLastState(string $version): ?string + { + $session = Session::getSession()->getNamespace('migrations'); + /** @var array $states */ + $states = $session->get($this->getModuleName(), []); + if (! isset($states[$version])) { + $schemaQuery = $this->getSchemaQueryFor($version); + $schemaQuery->setFilter(Filter::all(Filter::equal('success', 'n'))); + if (static::getColumnType($this->getDb(), $schemaQuery->getModel()->getTableName(), 'reason')) { + /** @var Schema $schema */ + $schema = $schemaQuery->first(); + if ($schema && version_compare($schema->version, $version, '==')) { + return $schema->reason; + } + } + + return null; + } + + return $states[$version]; + } +} From 81c9e5cfc50788df7ca62506329a830bd126577d Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 2 Aug 2023 18:25:08 +0200 Subject: [PATCH 03/48] Introduce `MigrationManager` class --- .../Icinga/Application/MigrationManager.php | 246 ++++++++++++++++++ 1 file changed, 246 insertions(+) create mode 100644 library/Icinga/Application/MigrationManager.php diff --git a/library/Icinga/Application/MigrationManager.php b/library/Icinga/Application/MigrationManager.php new file mode 100644 index 000000000..b1065261e --- /dev/null +++ b/library/Icinga/Application/MigrationManager.php @@ -0,0 +1,246 @@ + All pending migration hooks */ + protected $pendingMigrations; + + /** @var MigrationManager */ + private static $instance; + + private function __construct() + { + } + + /** + * Get the instance of this manager + * + * @return $this + */ + public static function instance(): self + { + if (self::$instance === null) { + self::$instance = new self(); + } + + return self::$instance; + } + + /** + * Get all pending migrations + * + * @return array + */ + public function getPendingMigrations(): array + { + if ($this->pendingMigrations === null) { + $this->load(); + } + + return $this->pendingMigrations; + } + + /** + * Get whether there are any pending migrations + * + * @return bool + */ + public function hasPendingMigrations(): bool + { + return $this->count() > 0; + } + + public function hasMigrations(string $module): bool + { + if (! $this->hasPendingMigrations()) { + return false; + } + + return isset($this->getPendingMigrations()[$module]); + } + + /** + * Get pending migration matching the given module name + * + * @param string $module + * + * @return MigrationHook + * + * @throws NotFoundError When there are no pending PHP code migrations matching the given module name + */ + public function getMigration(string $module): MigrationHook + { + if (! $this->hasMigrations($module)) { + throw new NotFoundError('There are no pending migrations matching the given name: %s', $module); + } + + return $this->getPendingMigrations()[$module]; + } + + /** + * Get the number of all pending migrations + * + * @return int + */ + public function count(): int + { + return count($this->getPendingMigrations()); + } + + /** + * Apply all pending migrations matching the given migration module name + * + * @param string $module + * + * @return bool + */ + public function applyByName(string $module): bool + { + $migration = $this->getMigration($module); + if ($migration->isModule() && $this->hasMigrations(MigrationHook::DEFAULT_MODULE)) { + return false; + } + + return $this->apply($migration); + } + + /** + * Apply the given migration hook + * + * @param MigrationHook $hook + * + * @return bool + */ + public function apply(MigrationHook $hook): bool + { + if ($hook->isModule() && $this->hasMigrations(MigrationHook::DEFAULT_MODULE)) { + Logger::error( + 'Please apply the Icinga Web pending migration(s) first or apply all the migrations instead' + ); + + return false; + } + + if ($hook->run()) { + unset($this->pendingMigrations[$hook->getModuleName()]); + + Logger::info('Applied pending %s migrations successfully', $hook->getName()); + + return true; + } + + return false; + } + + /** + * Apply all pending modules/framework migrations + * + * @return bool + */ + public function applyAll(): bool + { + $default = MigrationHook::DEFAULT_MODULE; + if ($this->hasMigrations($default)) { + $migration = $this->getMigration($default); + if (! $this->apply($migration)) { + return false; + } + } + + $succeeded = true; + foreach ($this->getPendingMigrations() as $migration) { + if (! $this->apply($migration) && $succeeded) { + $succeeded = false; + } + } + + return $succeeded; + } + + /** + * Yield module and framework pending migrations separately + * + * @param bool $modules + * + * @return Generator + */ + public function yieldMigrations(bool $modules = false): Generator + { + foreach ($this->getPendingMigrations() as $migration) { + if ($modules === $migration->isModule()) { + yield $migration; + } + } + } + + protected function load(): void + { + $this->pendingMigrations = []; + + /** @var MigrationHook $hook */ + foreach (Hook::all('migration') as $hook) { + if (empty($hook->getMigrations())) { + continue; + } + + $this->pendingMigrations[$hook->getModuleName()] = $hook; + } + + ksort($this->pendingMigrations); + } + + /** + * Get all pending migrations as an array + * + * @return array + */ + public function toArray(): array + { + $framework = []; + $serialize = function (MigrationHook $hook): array { + $serialized = [ + 'name' => $hook->getName(), + 'module' => $hook->getModuleName(), + 'isModule' => $hook->isModule(), + 'migrated_version' => $hook->getVersion(), + 'migrations' => [] + ]; + + foreach ($hook->getMigrations() as $migration) { + $serialized['migrations'][$migration->getVersion()] = [ + 'path' => $migration->getScriptPath(), + 'error' => $migration->getLastState() + ]; + } + + return $serialized; + }; + + foreach ($this->yieldMigrations() as $migration) { + $framework[] = $serialize($migration); + } + + $modules = []; + foreach ($this->yieldMigrations(true) as $migration) { + $modules[] = $serialize($migration); + } + + return ['System' => $framework, 'Modules' => $modules]; + } +} From 2daa1447b7e86da8f9e0ee30e7f74168532f73ed Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Mon, 24 Jul 2023 13:14:00 +0200 Subject: [PATCH 04/48] Introduce `MigrationForm` class --- application/forms/MigrationForm.php | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 application/forms/MigrationForm.php diff --git a/application/forms/MigrationForm.php b/application/forms/MigrationForm.php new file mode 100644 index 000000000..b3a48e41a --- /dev/null +++ b/application/forms/MigrationForm.php @@ -0,0 +1,25 @@ + ['icinga-form', 'migration-form', 'icinga-controls'], + 'name' => 'migration-form' + ]; + + protected function assemble(): void + { + $this->addHtml($this->createUidElement()); + } +} From 85b63dd0673613c42af1cc6162a22b6003f54619 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Mon, 24 Jul 2023 13:45:04 +0200 Subject: [PATCH 05/48] Introduce migration `ListItem` & `ItemList` classes --- library/Icinga/Web/StyleSheet.php | 3 +- .../Web/Widget/ItemList/MigrationList.php | 130 ++++++++++++++++++ .../Web/Widget/ItemList/MigrationListItem.php | 85 ++++++++++++ .../ItemList/MigrationListItemMinimal.php | 128 +++++++++++++++++ public/css/icinga/pending-migration.less | 100 ++++++++++++++ 5 files changed, 445 insertions(+), 1 deletion(-) create mode 100644 library/Icinga/Web/Widget/ItemList/MigrationList.php create mode 100644 library/Icinga/Web/Widget/ItemList/MigrationListItem.php create mode 100644 library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php create mode 100644 public/css/icinga/pending-migration.less diff --git a/library/Icinga/Web/StyleSheet.php b/library/Icinga/Web/StyleSheet.php index 9ca6d9af7..65cbb9705 100644 --- a/library/Icinga/Web/StyleSheet.php +++ b/library/Icinga/Web/StyleSheet.php @@ -80,7 +80,8 @@ class StyleSheet 'css/icinga/modal.less', 'css/icinga/audit.less', 'css/icinga/health.less', - 'css/icinga/php-diff.less' + 'css/icinga/php-diff.less', + 'css/icinga/pending-migration.less', ]; /** diff --git a/library/Icinga/Web/Widget/ItemList/MigrationList.php b/library/Icinga/Web/Widget/ItemList/MigrationList.php new file mode 100644 index 000000000..bf529752a --- /dev/null +++ b/library/Icinga/Web/Widget/ItemList/MigrationList.php @@ -0,0 +1,130 @@ + 'item-list']; + + /** @var Generator */ + protected $data; + + /** @var ?MigrationForm */ + protected $migrationForm; + + /** @var bool Whether to render minimal migration list items */ + protected $minimal = true; + + /** + * Create a new migration list + * + * @param Generator|array $data + * + * @param ?MigrationForm $form + */ + public function __construct($data, MigrationForm $form = null) + { + parent::__construct($data); + + $this->migrationForm = $form; + } + + /** + * Set whether to render minimal migration list items + * + * @param bool $minimal + * + * @return $this + */ + public function setMinimal(bool $minimal): self + { + $this->minimal = $minimal; + + return $this; + } + + /** + * Get whether to render minimal migration list items + * + * @return bool + */ + public function isMinimal(): bool + { + return $this->minimal; + } + + protected function getItemClass(): string + { + if ($this->isMinimal()) { + return MigrationListItemMinimal::class; + } + + return MigrationListItem::class; + } + + protected function assemble(): void + { + $itemClass = $this->getItemClass(); + + /** @var MigrationHook $data */ + foreach ($this->data as $data) { + /** @var MigrationListItem|MigrationListItemMinimal $item */ + $item = new $itemClass($data, $this); + if ($item instanceof MigrationListItemMinimal && $this->migrationForm) { + $migrateButton = $this->migrationForm->createElement( + 'submit', + sprintf('migrate-%s', $data->getModuleName()), + [ + 'required' => false, + 'label' => $this->translate('Migrate'), + 'title' => sprintf( + $this->translatePlural( + 'Migrate %d pending migration', + 'Migrate all %d pending migrations', + $data->count() + ), + $data->count() + ) + ] + ); + + $mm = MigrationManager::instance(); + if ($data->isModule() && $mm->hasMigrations(MigrationHook::DEFAULT_MODULE)) { + $migrateButton->getAttributes() + ->set('disabled', true) + ->set( + 'title', + $this->translate( + 'Please apply all the pending migrations of Icinga Web first or use the apply all' + . ' button instead.' + ) + ); + } + + $this->migrationForm->registerElement($migrateButton); + + $item->setMigrateButton($migrateButton); + } + + $this->addHtml($item); + } + + if ($this->isEmpty()) { + $this->setTag('div'); + $this->addHtml(new EmptyState(t('No items found.'))); + } + } +} diff --git a/library/Icinga/Web/Widget/ItemList/MigrationListItem.php b/library/Icinga/Web/Widget/ItemList/MigrationListItem.php new file mode 100644 index 000000000..80ba930d5 --- /dev/null +++ b/library/Icinga/Web/Widget/ItemList/MigrationListItem.php @@ -0,0 +1,85 @@ +item->getLastState()) { + $visual->getAttributes()->add('class', 'upgrade-failed'); + $visual->addHtml(new Icon('circle-xmark')); + } + } + + protected function assembleTitle(BaseHtmlElement $title): void + { + $scriptPath = $this->item->getScriptPath(); + /** @var string $parentDirs */ + $parentDirs = substr($scriptPath, (int) strpos($scriptPath, 'schema')); + $parentDirs = substr($parentDirs, 0, strrpos($parentDirs, '/') + 1); + + $title->addHtml( + new HtmlElement('span', null, Text::create($parentDirs)), + new HtmlElement('strong', null, Text::create($this->item->getVersion() . '.sql')) + ); + + if ($this->item->getLastState()) { + $title->addHtml( + new HtmlElement( + 'span', + Attributes::create(['class' => 'upgrade-failed']), + Text::create($this->translate('Upgrade failed')) + ) + ); + } + } + + protected function assembleHeader(BaseHtmlElement $header): void + { + $header->addHtml($this->createTitle()); + } + + protected function assembleCaption(BaseHtmlElement $caption): void + { + if ($this->item->getDescription()) { + $caption->addHtml(Text::create($this->item->getDescription())); + } + } + + protected function assembleFooter(BaseHtmlElement $footer): void + { + if ($this->item->getLastState()) { + $footer->addHtml( + new HtmlElement( + 'section', + Attributes::create(['class' => 'caption']), + new HtmlElement('pre', null, new HtmlString(Html::escape($this->item->getLastState()))) + ) + ); + } + } + + protected function assembleMain(BaseHtmlElement $main): void + { + $main->addHtml($this->createHeader(), $this->createCaption()); + } +} diff --git a/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php b/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php new file mode 100644 index 000000000..7602b5947 --- /dev/null +++ b/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php @@ -0,0 +1,128 @@ +migrateButton = $migrateButton; + + return $this; + } + + protected function assembleTitle(BaseHtmlElement $title): void + { + $title->addHtml( + FormattedString::create( + t('%s ', ''), + HtmlElement::create('span', ['class' => 'subject'], $this->item->getName()) + ) + ); + } + + protected function assembleHeader(BaseHtmlElement $header): void + { + if ($this->migrateButton === null) { + throw new LogicException('Please set the migrate submit button beforehand'); + } + + $header->addHtml($this->createTitle()); + $header->addHtml($this->migrateButton); + } + + protected function assembleCaption(BaseHtmlElement $caption): void + { + $migrations = $this->item->getMigrations(); + /** @var DbMigration $migration */ + $migration = array_shift($migrations); + if ($migration->getLastState()) { + $caption->addHtml(Text::create($migration->getDescription())); + + $wrapper = new HtmlElement( + 'div', + Attributes::create([ + 'class' => ['errors-section', 'collapsible'], + 'data-visible-height' => 150, + ]) + ); + + $wrapper->addHtml( + new Icon('circle-xmark'), + $caption, + new HtmlElement('pre', null, new HtmlString(Html::escape($migration->getLastState()))) + ); + + $caption->prependWrapper($wrapper); + } + } + + protected function assembleFooter(BaseHtmlElement $footer): void + { + $footer->addHtml((new MigrationList($this->item->getLatestMigrations(3)))->setMinimal(false)); + if ($this->item->count() > 3) { + $footer->addHtml( + new Link( + sprintf($this->translate('Show all %d migrations'), $this->item->count()), + Url::fromPath( + 'migrations/migration', + [MigrationHook::MIGRATION_PARAM => $this->item->getModuleName()] + ), + [ + 'data-base-target' => '_next' + ] + ) + ); + } + } + + protected function assembleMain(BaseHtmlElement $main): void + { + $this->getAttributes()->add('class', 'minimal'); + + $main->addHtml($this->createHeader()); + $caption = $this->createCaption(); + if (! $caption->isEmpty()) { + $main->addHtml($caption); + } + + $footer = $this->createFooter(); + if ($footer) { + $main->addHtml($footer); + } + } +} diff --git a/public/css/icinga/pending-migration.less b/public/css/icinga/pending-migration.less new file mode 100644 index 000000000..64cd57c48 --- /dev/null +++ b/public/css/icinga/pending-migration.less @@ -0,0 +1,100 @@ +// Style + +.migrations { + .migration-details { + > span { + color: @text-color-light; + } + + summary { + color: @icinga-blue; + cursor: pointer; + + &::-webkit-details-marker { + display: none; // Disable safari default details marker + } + } + } + + .migration-form { + input[type="submit"] { + color: @icinga-blue; + background: @low-sat-blue; + border: none; + + &:not(:disabled):hover { + background: @icinga-blue; + color: @text-color-on-icinga-blue; + } + + &:disabled { + background: @disabled-gray; + color: @default-text-color-inverted; + } + } + } + + .modules-header { + input[type="submit"] { + &:not(:disabled) { + background: transparent; + } + } + } +} + +// Layout + +.pending-migrations-hint { + top: 15%; + right: 50%; + position: fixed; + transform: translate(50%, -50%); + text-align: center; + + > p { + font-size: 1.1em; + } + + > h2 { + font-size: 2em; + } +} + +.migrations { + .list-item { + align-items: baseline; + } + + .migration-list-control { + padding-bottom: 1em; + } + + .modules-header { + display: inline-flex; + flex-wrap: nowrap; + align-items: baseline; + + .migration-form { + margin-left: .5em; + } + } + + .migration-details { + font-size: 1.1em; + + &[open] summary { + margin-bottom: .5em; + } + } + + .migration-form { + input[type="submit"] { + padding: 0.25em 0.5em; + line-height: 1.75; + font-size: 1.1em; + + .rounded-corners(3px); + } + } +} From faaebaeffb62fcdd1c4203299afd5b8c84eb466d Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Mon, 24 Jul 2023 13:46:50 +0200 Subject: [PATCH 06/48] Forward failed requests for routes with pending migrations --- application/controllers/ErrorController.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/application/controllers/ErrorController.php b/application/controllers/ErrorController.php index 580015b25..a22b040b7 100644 --- a/application/controllers/ErrorController.php +++ b/application/controllers/ErrorController.php @@ -3,6 +3,8 @@ namespace Icinga\Controllers; +use Icinga\Application\Hook\MigrationHook; +use Icinga\Application\MigrationManager; use Icinga\Exception\IcingaException; use Zend_Controller_Plugin_ErrorHandler; use Icinga\Application\Icinga; @@ -91,6 +93,22 @@ class ErrorController extends ActionController $this->getResponse()->setHttpResponseCode(403); break; default: + $mm = MigrationManager::instance(); + $action = $this->getRequest()->getActionName(); + $controller = $this->getRequest()->getControllerName(); + if ($action !== 'hint' && $controller !== 'migrations' && $mm->hasMigrations($moduleName)) { + // The view renderer from IPL web doesn't render the HTML content set in the respective + // controller if the error_handler request param is set, as it doesn't support error + // rendering. Since this error handler isn't caused by the migrations controller, we can + // safely unset this. + $this->setParam('error_handler', null); + $this->forward('hint', 'migrations', 'default', [ + MigrationHook::MIGRATION_PARAM => $moduleName + ]); + + return; + } + $this->getResponse()->setHttpResponseCode(500); $module = $modules->hasLoaded($moduleName) ? $modules->getModule($moduleName) : null; Logger::error("%s\n%s", $exception, IcingaException::getConfidentialTraceAsString($exception)); From a9db85ed715178803966d61ce748f8e3713728f8 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Mon, 24 Jul 2023 13:47:44 +0200 Subject: [PATCH 07/48] Introduce `application/migrations` permission --- application/forms/Security/RoleForm.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/application/forms/Security/RoleForm.php b/application/forms/Security/RoleForm.php index 3bddbf81b..58387f7aa 100644 --- a/application/forms/Security/RoleForm.php +++ b/application/forms/Security/RoleForm.php @@ -581,6 +581,9 @@ class RoleForm extends RepositoryForm ], 'application/sessions' => [ 'description' => t('Allow to manage user sessions') + ], + 'application/migrations' => [ + 'description' => t('Allow to apply pending application migrations') ] ]; From 1da5487066d4f4fb63247485dff1998f83c3a8bb Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Mon, 24 Jul 2023 13:59:19 +0200 Subject: [PATCH 08/48] Introduce `MigrationsController` & add pending migrations list in about view --- .../controllers/MigrationsController.php | 245 ++++++++++++++++++ application/views/scripts/about/index.phtml | 28 ++ library/Icinga/Web/Navigation/ConfigMenu.php | 35 ++- phpstan-baseline.neon | 5 - public/css/icinga/about.less | 14 + 5 files changed, 316 insertions(+), 11 deletions(-) create mode 100644 application/controllers/MigrationsController.php diff --git a/application/controllers/MigrationsController.php b/application/controllers/MigrationsController.php new file mode 100644 index 000000000..ca2c83f4c --- /dev/null +++ b/application/controllers/MigrationsController.php @@ -0,0 +1,245 @@ +getTabs()->extend(new OutputFormat(['csv'])); + $this->addTitleTab($this->translate('Migrations')); + + $canApply = $this->hasPermission('application/migrations'); + if (! $canApply) { + $this->addControl( + new HtmlElement( + 'div', + Attributes::create(['class' => 'migration-state-banner']), + new HtmlElement( + 'span', + null, + Text::create( + $this->translate('You do not have the required permission to apply pending migrations.') + ) + ) + ) + ); + } + + $migrateListForm = new MigrationForm(); + $migrateGlobalForm = new MigrationForm(); + $migrateGlobalForm->getAttributes()->set('name', sprintf('migrate-%s', MigrationHook::ALL_MIGRATIONS)); + + if ($canApply && $mm->hasPendingMigrations()) { + $migrateGlobalForm->addElement('submit', sprintf('migrate-%s', MigrationHook::ALL_MIGRATIONS), [ + 'required' => true, + 'label' => $this->translate('Migrate All'), + 'title' => $this->translate('Migrate all pending migrations') + ]); + + $this->controls->getAttributes()->add('class', 'default-layout'); + $this->handleMigrateRequest($migrateGlobalForm); + + $this->addControl($migrateGlobalForm); + } + + $this->handleFormatRequest($mm->toArray()); + + $frameworkList = new MigrationList($mm->yieldMigrations(), $migrateListForm); + $frameworkListControl = new HtmlElement('div', Attributes::create(['class' => 'migration-list-control'])); + $frameworkListControl->addHtml(new HtmlElement('h2', null, Text::create($this->translate('System')))); + $frameworkListControl->addHtml($frameworkList); + + $moduleList = new MigrationList($mm->yieldMigrations(true), $migrateListForm); + $moduleListControl = new HtmlElement('div', Attributes::create(['class' => 'migration-list-control'])); + $moduleListControl->addHtml(new HtmlElement('h2', null, Text::create($this->translate('Modules')))); + $moduleListControl->addHtml($moduleList); + + $migrateListForm->addHtml($frameworkListControl, $moduleListControl); + if ($canApply && $mm->hasPendingMigrations()) { + $frameworkList->ensureAssembled(); + $moduleList->ensureAssembled(); + + $this->handleMigrateRequest($migrateListForm); + } + + $migrations = new HtmlElement('div', Attributes::create(['class' => 'migrations'])); + $migrations->addHtml($migrateListForm); + + $this->addContent($migrations); + } + + public function hintAction(): void + { + // The forwarded request doesn't modify the original server query string, but adds the migration param to the + // request param instead. So, there is no way to access the migration param other than via the request instance. + /** @var ?string $module */ + $module = $this->getRequest()->getParam(MigrationHook::MIGRATION_PARAM); + if ($module === null) { + throw new MissingParameterException(t('Required parameter \'%s\' missing'), MigrationHook::MIGRATION_PARAM); + } + + $mm = MigrationManager::instance(); + if (! $mm->hasMigrations($module)) { + $this->httpNotFound(sprintf('There are no pending migrations matching the given name: %s', $module)); + } + + $migration = $mm->getMigration($module); + $this->addTitleTab($this->translate('Error')); + $this->addContent( + new HtmlElement( + 'div', + Attributes::create(['class' => 'pending-migrations-hint']), + new HtmlElement('h2', null, Text::create($this->translate('Error!'))), + new HtmlElement( + 'p', + null, + Text::create(sprintf($this->translate('%s has pending migrations.'), $migration->getName())) + ), + new HtmlElement('p', null, Text::create($this->translate('Please apply the migrations first.'))), + new ActionLink($this->translate('View pending Migrations'), 'migrations') + ) + ); + } + + public function migrationAction(): void + { + /** @var string $name */ + $name = $this->params->getRequired(MigrationHook::MIGRATION_PARAM); + + $this->addTitleTab($this->translate('Migration')); + $this->getTabs()->disableLegacyExtensions(); + $this->controls->getAttributes()->add('class', 'default-layout'); + + $mm = MigrationManager::instance(); + if (! $mm->hasMigrations($name)) { + $migrations = []; + } else { + $hook = $mm->getMigration($name); + $migrations = array_reverse($hook->getMigrations()); + if (! $this->hasPermission('application/migrations')) { + $this->addControl( + new HtmlElement( + 'div', + Attributes::create(['class' => 'migration-state-banner']), + new HtmlElement( + 'span', + null, + Text::create( + $this->translate('You do not have the required permission to apply pending migrations.') + ) + ) + ) + ); + } else { + $migrateForm = (new MigrationForm()) + ->addElement( + 'submit', + sprintf('migrate-%s', $hook->getModuleName()), + [ + 'required' => true, + 'label' => $this->translate('Migrate'), + 'title' => sprintf( + $this->translatePlural( + 'Migrate %d pending migration', + 'Migrate all %d pending migrations', + $hook->count() + ), + $hook->count() + ) + ] + ); + + $migrateForm->getAttributes()->add('class', 'inline'); + $this->handleMigrateRequest($migrateForm); + + $this->addControl( + new HtmlElement( + 'div', + Attributes::create(['class' => 'migration-controls']), + new HtmlElement('span', null, Text::create($hook->getName())), + $migrateForm + ) + ); + } + } + + $migrationWidget = new HtmlElement('div', Attributes::create(['class' => 'migrations'])); + $migrationWidget->addHtml((new MigrationList($migrations))->setMinimal(false)); + $this->addContent($migrationWidget); + } + + public function handleMigrateRequest(MigrationForm $form): void + { + $this->assertPermission('application/migrations'); + + $form->on(MigrationForm::ON_SUCCESS, function (MigrationForm $form) { + $mm = MigrationManager::instance(); + + $pressedButton = $form->getPressedSubmitElement(); + if ($pressedButton) { + $name = substr($pressedButton->getName(), 8); + switch ($name) { + case MigrationHook::ALL_MIGRATIONS: + if ($mm->applyAll()) { + Notification::success($this->translate('Applied all migrations successfully')); + } else { + Notification::error( + $this->translate( + 'Applied migrations successfully. Though, one or more migration hooks failed to run.' + . ' See logs for details' + ) + ); + } + break; + default: + $migration = $mm->getMigration($name); + if ($mm->apply($migration)) { + Notification::success($this->translate('Applied pending migrations successfully')); + } else { + Notification::error( + $this->translate('Failed to apply pending migration(s). See logs for details') + ); + } + } + } + + $this->redirectNow('migrations'); + })->handleRequest($this->getServerRequest()); + } + + /** + * Handle exports + * + * @param array $data + */ + protected function handleFormatRequest(array $data): void + { + $formatJson = $this->params->get('format') === 'json'; + if (! $formatJson && ! $this->getRequest()->isApiRequest()) { + return; + } + + $this->getResponse() + ->json() + ->setSuccessData($data) + ->sendResponse(); + } +} diff --git a/application/views/scripts/about/index.phtml b/application/views/scripts/about/index.phtml index e80cd89c6..805e723e0 100644 --- a/application/views/scripts/about/index.phtml +++ b/application/views/scripts/about/index.phtml @@ -1,7 +1,10 @@
@@ -93,6 +96,31 @@ use ipl\Web\Widget\Icon;
+ hasPendingMigrations()): ?> +
+

translate('Pending Migrations') ?>

+ + getPendingMigrations() as $migration): ?> + + + + + +
escape($migration->getName()) ?>getMigrations()), + BadgeNavigationItemRenderer::STATE_PENDING + ); + ?>
+ qlink( + $this->translate('Show all'), + 'migrations', + null, + ['title' => $this->translate('Show all pending migrations')] + ) ?> +
+ +

translate('Loaded Libraries') ?>

diff --git a/library/Icinga/Web/Navigation/ConfigMenu.php b/library/Icinga/Web/Navigation/ConfigMenu.php index ba541f721..333144a92 100644 --- a/library/Icinga/Web/Navigation/ConfigMenu.php +++ b/library/Icinga/Web/Navigation/ConfigMenu.php @@ -6,7 +6,9 @@ namespace Icinga\Web\Navigation; use Icinga\Application\Hook\HealthHook; use Icinga\Application\Icinga; use Icinga\Application\Logger; +use Icinga\Application\MigrationManager; use Icinga\Authentication\Auth; +use Icinga\Web\Navigation\Renderer\BadgeNavigationItemRenderer; use ipl\Html\Attributes; use ipl\Html\BaseHtmlElement; use ipl\Html\HtmlElement; @@ -47,6 +49,10 @@ class ConfigMenu extends BaseHtmlElement 'label' => t('Health'), 'url' => 'health', ], + 'migrations' => [ + 'label' => t('Migrations'), + 'url' => 'migrations', + ], 'announcements' => [ 'label' => t('Announcements'), 'url' => 'announcements' @@ -136,7 +142,7 @@ class ConfigMenu extends BaseHtmlElement null, [ new Icon('cog'), - $this->createHealthBadge(), + $this->createHealthBadge() ?? $this->createMigrationBadge(), ] ), $this->createLevel2Menu() @@ -211,7 +217,7 @@ class ConfigMenu extends BaseHtmlElement return false; } - protected function createHealthBadge() + protected function createHealthBadge(): ?StateBadge { $stateBadge = null; if ($this->getHealthCount() > 0) { @@ -222,6 +228,20 @@ class ConfigMenu extends BaseHtmlElement return $stateBadge; } + protected function createMigrationBadge(): ?StateBadge + { + $mm = MigrationManager::instance(); + $count = $mm->count(); + + $stateBadge = null; + if ($count > 0) { + $stateBadge = new StateBadge($count, BadgeNavigationItemRenderer::STATE_PENDING); + $stateBadge->addAttributes(['class' => 'disabled']); + } + + return $stateBadge; + } + protected function createLevel2Menu() { $level2Nav = HtmlElement::create( @@ -240,23 +260,26 @@ class ConfigMenu extends BaseHtmlElement return null; } - $healthBadge = null; + $stateBadge = null; $class = null; if ($key === 'health') { $class = 'badge-nav-item'; - $healthBadge = $this->createHealthBadge(); + $stateBadge = $this->createHealthBadge(); + } elseif ($key === 'migrations') { + $class = 'badge-nav-item'; + $stateBadge = $this->createMigrationBadge(); } $li = HtmlElement::create( 'li', - isset($item['atts']) ? $item['atts'] : [], + $item['atts'] ?? [], [ HtmlElement::create( 'a', Attributes::create(['href' => Url::fromPath($item['url'])]), [ $item['label'], - isset($healthBadge) ? $healthBadge : '' + $stateBadge ?? '' ] ), ] diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 932886e9a..25632c315 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -13165,11 +13165,6 @@ parameters: count: 1 path: library/Icinga/Web/Navigation/ConfigMenu.php - - - message: "#^Method Icinga\\\\Web\\\\Navigation\\\\ConfigMenu\\:\\:createHealthBadge\\(\\) has no return type specified\\.$#" - count: 1 - path: library/Icinga/Web/Navigation/ConfigMenu.php - - message: "#^Method Icinga\\\\Web\\\\Navigation\\\\ConfigMenu\\:\\:createLevel2Menu\\(\\) has no return type specified\\.$#" count: 1 diff --git a/public/css/icinga/about.less b/public/css/icinga/about.less index 4ccaaf8d1..fe3a71b71 100644 --- a/public/css/icinga/about.less +++ b/public/css/icinga/about.less @@ -13,6 +13,20 @@ > * { margin-bottom: 2em; } + + .pending-migrations { + .name-value-table.migrations { + tr:not(:first-child):not(:last-child) { + border-top: 1px solid @gray-lighter; + } + } + + a { + float: right; + margin-top: 1em; + color: @icinga-blue + } + } } h2 { From bc3c444cf5ad8a412ea600032218815694d75588 Mon Sep 17 00:00:00 2001 From: Florian Strohmaier Date: Thu, 27 Jul 2023 16:43:51 +0200 Subject: [PATCH 09/48] CSS: Adjust styles --- public/css/icinga/pending-migration.less | 81 ++++++++++++++---------- 1 file changed, 49 insertions(+), 32 deletions(-) diff --git a/public/css/icinga/pending-migration.less b/public/css/icinga/pending-migration.less index 64cd57c48..4cead154e 100644 --- a/public/css/icinga/pending-migration.less +++ b/public/css/icinga/pending-migration.less @@ -9,6 +9,7 @@ summary { color: @icinga-blue; cursor: pointer; + border-radius: .25em; &::-webkit-details-marker { display: none; // Disable safari default details marker @@ -17,27 +18,21 @@ } .migration-form { - input[type="submit"] { - color: @icinga-blue; - background: @low-sat-blue; - border: none; - - &:not(:disabled):hover { - background: @icinga-blue; - color: @text-color-on-icinga-blue; - } - - &:disabled { - background: @disabled-gray; - color: @default-text-color-inverted; - } - } } .modules-header { input[type="submit"] { - &:not(:disabled) { - background: transparent; + border: none; + padding-left: 0; + padding-right: 0; + + &:disabled { + color: @disabled-gray; + background: none; + + &:hover { + cursor: not-allowed; + } } } } @@ -46,16 +41,8 @@ // Layout .pending-migrations-hint { - top: 15%; - right: 50%; - position: fixed; - transform: translate(50%, -50%); text-align: center; - > p { - font-size: 1.1em; - } - > h2 { font-size: 2em; } @@ -66,6 +53,32 @@ align-items: baseline; } + .list-item header { + align-items: baseline; + height: 1.5em; + + input[type="submit"] { + margin-top: -.5em; + } + } + + .list-item .caption { + height: 1.5em; + margin-top: .25em; + + p { + margin-bottom: 0; + } + + .description { + line-height: 1.5; + } + } + + footer { + margin-top: .25em; + } + .migration-list-control { padding-bottom: 1em; } @@ -81,20 +94,24 @@ } .migration-details { - font-size: 1.1em; + font-size: 1em; + line-height: 1.5em; - &[open] summary { - margin-bottom: .5em; + summary { + display: inline-block; } } .migration-form { input[type="submit"] { - padding: 0.25em 0.5em; - line-height: 1.75; - font-size: 1.1em; + line-height: 1.5; - .rounded-corners(3px); + &:disabled { + color: @disabled-gray; + cursor: not-allowed; + background: none; + border-color: fade(@disabled-gray, 75) + } } } } From 15792fb59a4ee3ee878749e1b90012691ca41251 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 2 Aug 2023 18:27:55 +0200 Subject: [PATCH 10/48] Provide `DbMigration` hook & register when bootstrapping --- .../Application/ApplicationBootstrap.php | 13 ++++ library/Icinga/Application/Cli.php | 3 +- library/Icinga/Application/EmbeddedWeb.php | 3 +- .../Application/ProvidedHook/DbMigration.php | 73 +++++++++++++++++++ library/Icinga/Application/Web.php | 3 +- library/Icinga/Common/Database.php | 2 +- 6 files changed, 93 insertions(+), 4 deletions(-) create mode 100644 library/Icinga/Application/ProvidedHook/DbMigration.php diff --git a/library/Icinga/Application/ApplicationBootstrap.php b/library/Icinga/Application/ApplicationBootstrap.php index 4417f2750..ecc283e65 100644 --- a/library/Icinga/Application/ApplicationBootstrap.php +++ b/library/Icinga/Application/ApplicationBootstrap.php @@ -6,6 +6,7 @@ namespace Icinga\Application; use DirectoryIterator; use ErrorException; use Exception; +use Icinga\Application\ProvidedHook\DbMigration; use ipl\I18n\GettextTranslator; use ipl\I18n\StaticTranslator; use LogicException; @@ -731,4 +732,16 @@ abstract class ApplicationBootstrap $localedir = $this->getLocaleDir(); return $localedir !== false && file_exists($localedir) && is_dir($localedir); } + + /** + * Register all hooks provided by the main application + * + * @return $this + */ + protected function registerApplicationHooks(): self + { + Hook::register('migration', DbMigration::class, DbMigration::class); + + return $this; + } } diff --git a/library/Icinga/Application/Cli.php b/library/Icinga/Application/Cli.php index 28109f94f..3b937382c 100644 --- a/library/Icinga/Application/Cli.php +++ b/library/Icinga/Application/Cli.php @@ -48,7 +48,8 @@ class Cli extends ApplicationBootstrap ->setupModuleManager() ->setupUserBackendFactory() ->loadSetupModuleIfNecessary() - ->setupFakeAuthentication(); + ->setupFakeAuthentication() + ->registerApplicationHooks(); } /** diff --git a/library/Icinga/Application/EmbeddedWeb.php b/library/Icinga/Application/EmbeddedWeb.php index 8d03e1133..9adb3a429 100644 --- a/library/Icinga/Application/EmbeddedWeb.php +++ b/library/Icinga/Application/EmbeddedWeb.php @@ -75,7 +75,8 @@ class EmbeddedWeb extends ApplicationBootstrap ->setupTimezone() ->prepareFakeInternationalization() ->setupModuleManager() - ->loadEnabledModules(); + ->loadEnabledModules() + ->registerApplicationHooks(); } /** diff --git a/library/Icinga/Application/ProvidedHook/DbMigration.php b/library/Icinga/Application/ProvidedHook/DbMigration.php new file mode 100644 index 000000000..20054b790 --- /dev/null +++ b/library/Icinga/Application/ProvidedHook/DbMigration.php @@ -0,0 +1,73 @@ +translate('Icinga Web 2'); + } + + public function providedDescriptions(): array + { + return []; + } + + public function getVersion(): string + { + if ($this->version === null) { + $conn = $this->getDb(); + $schemaQuery = Schema::on($conn) + ->orderBy('id', SORT_DESC) + ->limit(2); + + if (static::getColumnType($conn, $schemaQuery->getModel()->getTableName(), 'success')) { + /** @var Schema $schema */ + foreach ($schemaQuery as $schema) { + if ($schema->success) { + $this->version = $schema->version; + } + } + + if (! $this->version) { + $this->version = '2.12.0'; + } + } elseif (static::tableExists($conn, $schemaQuery->getModel()->getTableName())) { + $this->version = '2.11.0'; + } elseif (static::tableExists($conn, 'icingaweb_rememberme')) { + $randomIvType = static::getColumnType($conn, 'icingaweb_rememberme', 'random_iv'); + if ($randomIvType === 'varchar(32)') { + $this->version = '2.9.1'; + } else { + $this->version = '2.9.0'; + } + } else { + $usernameType = static::getColumnType($conn, 'icingaweb_group_membership', 'username'); + if ($usernameType === 'varchar(254)') { + $this->version = '2.5.0'; + } else { + $this->version = '2.0.0'; + } + } + } + + return $this->version; + } + + protected function getSchemaQueryFor(string $version): Query + { + return Schema::on($this->getDb()) + ->filter(Filter::equal('version', $version)); + } +} diff --git a/library/Icinga/Application/Web.php b/library/Icinga/Application/Web.php index 268943ff8..934af0745 100644 --- a/library/Icinga/Application/Web.php +++ b/library/Icinga/Application/Web.php @@ -104,7 +104,8 @@ class Web extends EmbeddedWeb ->setupUser() ->setupTimezone() ->setupInternationalization() - ->setupFatalErrorHandling(); + ->setupFatalErrorHandling() + ->registerApplicationHooks(); } /** diff --git a/library/Icinga/Common/Database.php b/library/Icinga/Common/Database.php index 4c977653c..d54eb253b 100644 --- a/library/Icinga/Common/Database.php +++ b/library/Icinga/Common/Database.php @@ -22,7 +22,7 @@ trait Database * * @throws \Icinga\Exception\ConfigurationError */ - protected function getDb() + protected function getDb(): Connection { if (! $this->hasDb()) { throw new LogicException('Please check if a db instance exists at all'); From dec24686bc48bef37e0bed61809eb4a4d1bf2066 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 2 Aug 2023 18:28:59 +0200 Subject: [PATCH 11/48] Pending migrations CSS enhancement --- public/css/icinga/pending-migration.less | 150 ++++++++++++----------- 1 file changed, 79 insertions(+), 71 deletions(-) diff --git a/public/css/icinga/pending-migration.less b/public/css/icinga/pending-migration.less index 4cead154e..8c574c9f5 100644 --- a/public/css/icinga/pending-migration.less +++ b/public/css/icinga/pending-migration.less @@ -1,38 +1,34 @@ // Style +.migration-state-banner { + .rounded-corners(); + + border: 1px solid @gray-lighter; + color: @text-color-light; + padding: 1em; + text-align: center; +} + .migrations { - .migration-details { - > span { - color: @text-color-light; - } + a { + color: @icinga-blue; + } - summary { - color: @icinga-blue; - cursor: pointer; - border-radius: .25em; - - &::-webkit-details-marker { - display: none; // Disable safari default details marker - } + .list-item { + .visual.upgrade-failed, span.upgrade-failed, .errors-section > i { + color: @state-critical; } } - .migration-form { - } - - .modules-header { - input[type="submit"] { - border: none; - padding-left: 0; - padding-right: 0; +.migration-form { + input[type="submit"] { + line-height: 1.5; &:disabled { color: @disabled-gray; + cursor: not-allowed; background: none; - - &:hover { - cursor: not-allowed; - } + border-color: fade(@disabled-gray, 75) } } } @@ -48,69 +44,81 @@ } } +.migration-controls { + display: flex; + align-items: center; + justify-content: space-between; +} + .migrations { - .list-item { - align-items: baseline; - } - - .list-item header { - align-items: baseline; - height: 1.5em; - - input[type="submit"] { - margin-top: -.5em; - } - } - - .list-item .caption { - height: 1.5em; - margin-top: .25em; - - p { - margin-bottom: 0; - } - - .description { - line-height: 1.5; - } - } - - footer { - margin-top: .25em; + .icinga-form { // Reset Icinga Form layout styles + width: unset; + max-width: unset; } .migration-list-control { padding-bottom: 1em; + + > .item-list { + max-width: 50em; + } } - .modules-header { - display: inline-flex; - flex-wrap: nowrap; + .list-item { align-items: baseline; - .migration-form { - margin-left: .5em; + .visual.upgrade-failed + .main { + margin-left: 0; } - } - .migration-details { - font-size: 1em; - line-height: 1.5em; + header { + align-items: baseline; - summary { + .title span.upgrade-failed { + margin: .5em; + } + } + + .caption, .errors-section pre { + margin-top: .25em; + height: auto; + -webkit-line-clamp: 3; + } + + .errors-section { + margin-top: 1em; + border: 1px solid @state-critical; + .rounded-corners(); + + i.icon { + position: absolute; + margin-top: .5em; + margin-left: .5em; + } + + .caption { + margin-left: 1.8em; + } + } + + &.minimal footer { display: inline-block; } - } - .migration-form { - input[type="submit"] { - line-height: 1.5; + footer { + width: 100%; + padding-top: 0; - &:disabled { - color: @disabled-gray; - cursor: not-allowed; - background: none; - border-color: fade(@disabled-gray, 75) + > * { + font-size: 1em; + } + + .list-item:first-child .main { + padding-top: 0; + } + + a { + margin-left: .5em; } } } From ce012dcdb2edc80ac78da7583da06fdcf2de6c88 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Mon, 7 Aug 2023 14:12:32 +0200 Subject: [PATCH 12/48] Hook: Don't abort loading remaining hooks due to one broken hook `Hook::all()` shouldn't abort loading the remaining hooks when one of the provided hook is broken. --- library/Icinga/Application/Hook.php | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/library/Icinga/Application/Hook.php b/library/Icinga/Application/Hook.php index dcfd5dd84..9720c6ab6 100644 --- a/library/Icinga/Application/Hook.php +++ b/library/Icinga/Application/Hook.php @@ -266,23 +266,21 @@ class Hook * * @return array */ - public static function all($name) + public static function all($name): array { $name = self::normalizeHookName($name); if (! self::has($name)) { - return array(); + return []; } foreach (self::$hooks[$name] as $key => $hook) { list($class, $alwaysRun) = $hook; if ($alwaysRun || self::hasPermission($class)) { - if (self::createInstance($name, $key) === null) { - return array(); - } + self::createInstance($name, $key); } } - return isset(self::$instances[$name]) ? self::$instances[$name] : array(); + return self::$instances[$name] ?? []; } /** From d186604b629eeafbf66bf0da67e92223a38fd858 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 30 Aug 2023 10:55:58 +0200 Subject: [PATCH 13/48] Allow to define row count after which a collapsible can be collapsed --- public/js/icinga/behavior/collapsible.js | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/public/js/icinga/behavior/collapsible.js b/public/js/icinga/behavior/collapsible.js index c0b5349e3..16f719506 100644 --- a/public/js/icinga/behavior/collapsible.js +++ b/public/js/icinga/behavior/collapsible.js @@ -320,14 +320,21 @@ let rowSelector = this.getRowSelector(collapsible); if (!! rowSelector) { - let visibleRows = Number(collapsible.dataset.visibleRows); - if (isNaN(visibleRows)) { - visibleRows = this.defaultVisibleRows; - } else if (visibleRows === 0) { + let collapseAfter = Number(collapsible.dataset.collapseAfter) + if (isNaN(collapseAfter)) { + collapseAfter = Number(collapsible.dataset.visibleRows); + if (isNaN(collapseAfter)) { + collapseAfter = this.defaultVisibleRows; + } + + collapseAfter *= 2; + } + + if (collapseAfter === 0) { return true; } - return collapsible.querySelectorAll(rowSelector).length > visibleRows * 2; + return collapsible.querySelectorAll(rowSelector).length > collapseAfter; } else { let maxHeight = Number(collapsible.dataset.visibleHeight); if (isNaN(maxHeight)) { From fb33a2097a58fcb23ed4addfb4d4aa56106c3481 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Mon, 11 Sep 2023 15:06:25 +0200 Subject: [PATCH 14/48] Defferentiate migrations with no provided descriptions --- library/Icinga/Web/Widget/ItemList/MigrationListItem.php | 3 +++ .../Web/Widget/ItemList/MigrationListItemMinimal.php | 7 ++++++- public/css/icinga/pending-migration.less | 4 ++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/library/Icinga/Web/Widget/ItemList/MigrationListItem.php b/library/Icinga/Web/Widget/ItemList/MigrationListItem.php index 80ba930d5..d75d179d6 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationListItem.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationListItem.php @@ -62,6 +62,9 @@ class MigrationListItem extends BaseListItem { if ($this->item->getDescription()) { $caption->addHtml(Text::create($this->item->getDescription())); + } else { + $caption->getAttributes()->add('class', 'empty-state'); + $caption->addHtml(Text::create($this->translate('No description provided.'))); } } diff --git a/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php b/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php index 7602b5947..616c06d40 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php @@ -71,7 +71,12 @@ class MigrationListItemMinimal extends BaseListItem /** @var DbMigration $migration */ $migration = array_shift($migrations); if ($migration->getLastState()) { - $caption->addHtml(Text::create($migration->getDescription())); + if ($migration->getDescription()) { + $caption->addHtml(Text::create($migration->getDescription())); + } else { + $caption->getAttributes()->add('class', 'empty-state'); + $caption->addHtml(Text::create($this->translate('No description provided.'))); + } $wrapper = new HtmlElement( 'div', diff --git a/public/css/icinga/pending-migration.less b/public/css/icinga/pending-migration.less index 8c574c9f5..581a888fc 100644 --- a/public/css/icinga/pending-migration.less +++ b/public/css/icinga/pending-migration.less @@ -18,6 +18,10 @@ .visual.upgrade-failed, span.upgrade-failed, .errors-section > i { color: @state-critical; } + + .caption.empty-state { + color: @gray-semilight; + } } .migration-form { From 192a21b668ae65398586b27baad06742e91cc34b Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Mon, 11 Sep 2023 15:52:17 +0200 Subject: [PATCH 15/48] Don't use `strong` tag to highlight unselectable items --- library/Icinga/Web/Widget/ItemList/MigrationListItem.php | 6 +++++- public/css/icinga/pending-migration.less | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/library/Icinga/Web/Widget/ItemList/MigrationListItem.php b/library/Icinga/Web/Widget/ItemList/MigrationListItem.php index d75d179d6..ee0f03fef 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationListItem.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationListItem.php @@ -39,7 +39,11 @@ class MigrationListItem extends BaseListItem $title->addHtml( new HtmlElement('span', null, Text::create($parentDirs)), - new HtmlElement('strong', null, Text::create($this->item->getVersion() . '.sql')) + new HtmlElement( + 'span', + Attributes::create(['class' => 'version']), + Text::create($this->item->getVersion() . '.sql') + ) ); if ($this->item->getLastState()) { diff --git a/public/css/icinga/pending-migration.less b/public/css/icinga/pending-migration.less index 581a888fc..518e353e4 100644 --- a/public/css/icinga/pending-migration.less +++ b/public/css/icinga/pending-migration.less @@ -22,6 +22,10 @@ .caption.empty-state { color: @gray-semilight; } + + span.version { + color: @text-color; + } } .migration-form { From fdadba59ca1239b0a0beafdd47fb9e1cf21199bc Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Mon, 11 Sep 2023 15:52:56 +0200 Subject: [PATCH 16/48] Fix form with mulitple buttons doesn't recognize whether it's been submitted --- application/forms/MigrationForm.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/application/forms/MigrationForm.php b/application/forms/MigrationForm.php index b3a48e41a..940479153 100644 --- a/application/forms/MigrationForm.php +++ b/application/forms/MigrationForm.php @@ -18,6 +18,16 @@ class MigrationForm extends Form 'name' => 'migration-form' ]; + public function hasBeenSubmitted(): bool + { + if (! $this->hasBeenSent()) { + return false; + } + + $pressedButton = $this->getPressedSubmitElement(); + return $pressedButton && strpos($pressedButton->getName(), 'migrate-') !== false; + } + protected function assemble(): void { $this->addHtml($this->createUidElement()); From ad02431bd1c179d0ffc98eaf89dd30e6e0640575 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Mon, 11 Sep 2023 17:15:13 +0200 Subject: [PATCH 17/48] Add extra `class` to outer item lists & render subject header in the error box --- .../Web/Widget/ItemList/MigrationList.php | 3 +++ .../ItemList/MigrationListItemMinimal.php | 23 +++++++++++++++++-- public/css/icinga/pending-migration.less | 12 +++++----- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/library/Icinga/Web/Widget/ItemList/MigrationList.php b/library/Icinga/Web/Widget/ItemList/MigrationList.php index bf529752a..27f9b6eb9 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationList.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationList.php @@ -78,6 +78,9 @@ class MigrationList extends BaseItemList protected function assemble(): void { $itemClass = $this->getItemClass(); + if (! $this->isMinimal()) { + $this->getAttributes()->add('class', 'file-list'); + } /** @var MigrationHook $data */ foreach ($this->data as $data) { diff --git a/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php b/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php index 616c06d40..2a8966cc3 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php @@ -86,8 +86,29 @@ class MigrationListItemMinimal extends BaseListItem ]) ); + $scriptPath = $migration->getScriptPath(); + /** @var string $parentDirs */ + $parentDirs = substr($scriptPath, (int) strpos($scriptPath, 'schema')); + $parentDirs = substr($parentDirs, 0, strrpos($parentDirs, '/') + 1); + + $title = new HtmlElement('div', Attributes::create(['class' => 'title'])); + $title->addHtml( + new HtmlElement('span', null, Text::create($parentDirs)), + new HtmlElement( + 'span', + Attributes::create(['class' => 'version']), + Text::create($migration->getVersion() . '.sql') + ), + new HtmlElement( + 'span', + Attributes::create(['class' => 'upgrade-failed']), + Text::create($this->translate('Upgrade failed')) + ) + ); + $wrapper->addHtml( new Icon('circle-xmark'), + new HtmlElement('header', null, $title), $caption, new HtmlElement('pre', null, new HtmlString(Html::escape($migration->getLastState()))) ); @@ -117,8 +138,6 @@ class MigrationListItemMinimal extends BaseListItem protected function assembleMain(BaseHtmlElement $main): void { - $this->getAttributes()->add('class', 'minimal'); - $main->addHtml($this->createHeader()); $caption = $this->createCaption(); if (! $caption->isEmpty()) { diff --git a/public/css/icinga/pending-migration.less b/public/css/icinga/pending-migration.less index 518e353e4..9ee935be9 100644 --- a/public/css/icinga/pending-migration.less +++ b/public/css/icinga/pending-migration.less @@ -72,6 +72,10 @@ } } + .item-list:not(.file-list) .list-item footer { + display: inline-block; + } + .list-item { align-items: baseline; @@ -100,19 +104,15 @@ i.icon { position: absolute; - margin-top: .5em; + margin-top: .3em; margin-left: .5em; } - .caption { + .caption, header { margin-left: 1.8em; } } - &.minimal footer { - display: inline-block; - } - footer { width: 100%; padding-top: 0; From a00f094e1073543291595edd2654f1c10b4e827f Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Tue, 12 Sep 2023 11:01:36 +0200 Subject: [PATCH 18/48] Add extra collapsible container around error section --- .../ItemList/MigrationListItemMinimal.php | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php b/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php index 2a8966cc3..05d5cd973 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php @@ -78,14 +78,6 @@ class MigrationListItemMinimal extends BaseListItem $caption->addHtml(Text::create($this->translate('No description provided.'))); } - $wrapper = new HtmlElement( - 'div', - Attributes::create([ - 'class' => ['errors-section', 'collapsible'], - 'data-visible-height' => 150, - ]) - ); - $scriptPath = $migration->getScriptPath(); /** @var string $parentDirs */ $parentDirs = substr($scriptPath, (int) strpos($scriptPath, 'schema')); @@ -106,14 +98,21 @@ class MigrationListItemMinimal extends BaseListItem ) ); - $wrapper->addHtml( + $error = new HtmlElement('div', Attributes::create([ + 'class' => 'collapsible', + 'data-visible-height' => 150, + ])); + $error->addHtml(new HtmlElement('pre', null, new HtmlString(Html::escape($migration->getLastState())))); + + $errorSection = new HtmlElement('div', Attributes::create(['class' => 'errors-section',])); + $errorSection->addHtml( new Icon('circle-xmark'), new HtmlElement('header', null, $title), $caption, - new HtmlElement('pre', null, new HtmlString(Html::escape($migration->getLastState()))) + $error ); - $caption->prependWrapper($wrapper); + $caption->prependWrapper($errorSection); } } From 44897e490340a131b7ac31644be33d219d369bda Mon Sep 17 00:00:00 2001 From: Florian Strohmaier Date: Tue, 12 Sep 2023 11:20:21 +0200 Subject: [PATCH 19/48] CSS: Styling --- public/css/icinga/pending-migration.less | 53 +++++++++++++++++++----- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/public/css/icinga/pending-migration.less b/public/css/icinga/pending-migration.less index 9ee935be9..46ba09b11 100644 --- a/public/css/icinga/pending-migration.less +++ b/public/css/icinga/pending-migration.less @@ -1,5 +1,7 @@ // Style +@visual-width: 1.5em; + .migration-state-banner { .rounded-corners(); @@ -14,8 +16,12 @@ color: @icinga-blue; } + .empty-state { + margin: 0 auto; + } + .list-item { - .visual.upgrade-failed, span.upgrade-failed, .errors-section > i { + .visual.upgrade-failed, span.upgrade-failed, .errors-section > header > i { color: @state-critical; } @@ -43,7 +49,6 @@ } // Layout - .pending-migrations-hint { text-align: center; @@ -72,19 +77,30 @@ } } - .item-list:not(.file-list) .list-item footer { - display: inline-block; + .item-list:not(.file-list) > .list-item { + > .main { + border-top: none; + } + + footer { + display: block; + } } .list-item { align-items: baseline; - .visual.upgrade-failed + .main { + .main { margin-left: 0; } header { align-items: baseline; + justify-content: flex-start; + + input { + margin-left: auto; + } .title span.upgrade-failed { margin: .5em; @@ -98,14 +114,15 @@ } .errors-section { - margin-top: 1em; + margin: 1em -.25em; border: 1px solid @state-critical; - .rounded-corners(); + padding: .25em; + .rounded-corners(.5em); - i.icon { - position: absolute; + .status-icon { margin-top: .3em; - margin-left: .5em; + margin-left: -1.5em; + margin-right: .25em; } .caption, header { @@ -126,8 +143,22 @@ } a { - margin-left: .5em; + margin-left: @visual-width; } } } } + +.item-list.file-list { + .visual { + width: @visual-width; + } + + .main { + margin-left: @visual-width; + } + + .visual + .main { + margin-left: 0; + } +} From 7e313c921a951f1a202df057998bf4a9f3392279 Mon Sep 17 00:00:00 2001 From: Florian Strohmaier Date: Tue, 12 Sep 2023 11:20:40 +0200 Subject: [PATCH 20/48] MigrationListItemMinimal: Customize markup for styling --- .../Web/Widget/ItemList/MigrationListItemMinimal.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php b/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php index 05d5cd973..f4137e39c 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php @@ -100,14 +100,13 @@ class MigrationListItemMinimal extends BaseListItem $error = new HtmlElement('div', Attributes::create([ 'class' => 'collapsible', - 'data-visible-height' => 150, + 'data-visible-height' => '58', ])); $error->addHtml(new HtmlElement('pre', null, new HtmlString(Html::escape($migration->getLastState())))); $errorSection = new HtmlElement('div', Attributes::create(['class' => 'errors-section',])); $errorSection->addHtml( - new Icon('circle-xmark'), - new HtmlElement('header', null, $title), + new HtmlElement('header', null, new Icon('circle-xmark', ['class' => 'status-icon']), $title), $caption, $error ); @@ -128,7 +127,8 @@ class MigrationListItemMinimal extends BaseListItem [MigrationHook::MIGRATION_PARAM => $this->item->getModuleName()] ), [ - 'data-base-target' => '_next' + 'data-base-target' => '_next', + 'class' => 'show-more' ] ) ); From 2944ceaa52e3a05d8ff070fa4defcb400429b3fd Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Tue, 12 Sep 2023 16:57:53 +0200 Subject: [PATCH 21/48] Rename `getSchemaQueryFor()` & drop `$version` param --- .../Icinga/Application/Hook/MigrationHook.php | 19 +++++++++++-------- .../Application/ProvidedHook/DbMigration.php | 8 +++----- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/library/Icinga/Application/Hook/MigrationHook.php b/library/Icinga/Application/Hook/MigrationHook.php index bbbc529cc..9e31f3421 100644 --- a/library/Icinga/Application/Hook/MigrationHook.php +++ b/library/Icinga/Application/Hook/MigrationHook.php @@ -191,9 +191,12 @@ abstract class MigrationHook implements Countable ); Logger::debug($e->getTraceAsString()); + $schemaQuery = $this->getSchemaQuery() + ->filter(Filter::equal('version', $migration->getVersion())); + static::insertFailedEntry( $conn, - $this->getSchemaQueryFor($migration->getVersion()), + $schemaQuery, $migration->getVersion(), $e->getMessage() . PHP_EOL . $e->getTraceAsString() ); @@ -247,13 +250,11 @@ abstract class MigrationHook implements Countable abstract protected function getDb(): Connection; /** - * Get a schema version query filtered by the given $version - * - * @param string $version + * Get a schema version query * * @return Query */ - abstract protected function getSchemaQueryFor(string $version): Query; + abstract protected function getSchemaQuery(): Query; protected function load(): void { @@ -361,12 +362,14 @@ abstract class MigrationHook implements Countable /** @var array $states */ $states = $session->get($this->getModuleName(), []); if (! isset($states[$version])) { - $schemaQuery = $this->getSchemaQueryFor($version); - $schemaQuery->setFilter(Filter::all(Filter::equal('success', 'n'))); + $schemaQuery = $this->getSchemaQuery() + ->filter(Filter::equal('version', $version)) + ->filter(Filter::all(Filter::equal('success', 'n'))); + if (static::getColumnType($this->getDb(), $schemaQuery->getModel()->getTableName(), 'reason')) { /** @var Schema $schema */ $schema = $schemaQuery->first(); - if ($schema && version_compare($schema->version, $version, '==')) { + if ($schema) { return $schema->reason; } } diff --git a/library/Icinga/Application/ProvidedHook/DbMigration.php b/library/Icinga/Application/ProvidedHook/DbMigration.php index 20054b790..527b910c1 100644 --- a/library/Icinga/Application/ProvidedHook/DbMigration.php +++ b/library/Icinga/Application/ProvidedHook/DbMigration.php @@ -8,7 +8,6 @@ use Icinga\Application\Hook\MigrationHook; use Icinga\Common\Database; use Icinga\Model\Schema; use ipl\Orm\Query; -use ipl\Stdlib\Filter; class DbMigration extends MigrationHook { @@ -28,7 +27,7 @@ class DbMigration extends MigrationHook { if ($this->version === null) { $conn = $this->getDb(); - $schemaQuery = Schema::on($conn) + $schemaQuery = $this->getSchemaQuery() ->orderBy('id', SORT_DESC) ->limit(2); @@ -65,9 +64,8 @@ class DbMigration extends MigrationHook return $this->version; } - protected function getSchemaQueryFor(string $version): Query + protected function getSchemaQuery(): Query { - return Schema::on($this->getDb()) - ->filter(Filter::equal('version', $version)); + return Schema::on($this->getDb()); } } From 4b2784f85ee4cb24254636f4ce89b0a2548b5de3 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Tue, 12 Sep 2023 17:00:43 +0200 Subject: [PATCH 22/48] Use `Icinga Web` as a component name --- library/Icinga/Application/ProvidedHook/DbMigration.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/Icinga/Application/ProvidedHook/DbMigration.php b/library/Icinga/Application/ProvidedHook/DbMigration.php index 527b910c1..2f6ecc1be 100644 --- a/library/Icinga/Application/ProvidedHook/DbMigration.php +++ b/library/Icinga/Application/ProvidedHook/DbMigration.php @@ -15,7 +15,7 @@ class DbMigration extends MigrationHook public function getName(): string { - return $this->translate('Icinga Web 2'); + return $this->translate('Icinga Web'); } public function providedDescriptions(): array From 13569a34b7ae351a0f857fb6107010b2d7e7148b Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Tue, 12 Sep 2023 17:01:31 +0200 Subject: [PATCH 23/48] Check explicitly for `false` before raising an unknown error --- library/Icinga/Application/Hook/Common/DbMigration.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/Icinga/Application/Hook/Common/DbMigration.php b/library/Icinga/Application/Hook/Common/DbMigration.php index 14219be58..a115e10d1 100644 --- a/library/Icinga/Application/Hook/Common/DbMigration.php +++ b/library/Icinga/Application/Hook/Common/DbMigration.php @@ -126,10 +126,14 @@ class DbMigration { if (! $this->query) { $statements = @file_get_contents($this->getScriptPath()); - if (! $statements) { + if ($statements === false) { throw new RuntimeException(sprintf('Cannot load upgrade script %s', $this->getScriptPath())); } + if (empty($statements)) { + throw new RuntimeException('Nothing to migrate'); + } + if (preg_match('/\s*delimiter\s*(\S+)\s*$/im', $statements, $matches)) { /** @var string $statements */ $statements = preg_replace('/\s*delimiter\s*(\S+)\s*$/im', '', $statements); @@ -140,10 +144,6 @@ class DbMigration $this->query = $statements; } - if (empty($this->query)) { - throw new RuntimeException('Nothing to migrate'); - } - $conn->exec($this->query); return $this; From 821a6812ae82aca09d987784d13efffdbd91b6bf Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 13 Sep 2023 10:34:14 +0200 Subject: [PATCH 24/48] Use `EmptyState(Bar)` classes where applicable --- library/Icinga/Web/Widget/ItemList/MigrationList.php | 4 ++-- library/Icinga/Web/Widget/ItemList/MigrationListItem.php | 4 ++-- .../Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php | 4 ++-- public/css/icinga/pending-migration.less | 4 ---- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/library/Icinga/Web/Widget/ItemList/MigrationList.php b/library/Icinga/Web/Widget/ItemList/MigrationList.php index 27f9b6eb9..d1836cd98 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationList.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationList.php @@ -11,7 +11,7 @@ use Icinga\Application\MigrationManager; use Icinga\Forms\MigrationForm; use ipl\I18n\Translation; use ipl\Web\Common\BaseItemList; -use ipl\Web\Widget\EmptyState; +use ipl\Web\Widget\EmptyStateBar; class MigrationList extends BaseItemList { @@ -127,7 +127,7 @@ class MigrationList extends BaseItemList if ($this->isEmpty()) { $this->setTag('div'); - $this->addHtml(new EmptyState(t('No items found.'))); + $this->addHtml(new EmptyStateBar(t('No items found.'))); } } } diff --git a/library/Icinga/Web/Widget/ItemList/MigrationListItem.php b/library/Icinga/Web/Widget/ItemList/MigrationListItem.php index ee0f03fef..89ceade82 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationListItem.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationListItem.php @@ -13,6 +13,7 @@ use ipl\Html\HtmlString; use ipl\Html\Text; use ipl\I18n\Translation; use ipl\Web\Common\BaseListItem; +use ipl\Web\Widget\EmptyState; use ipl\Web\Widget\Icon; class MigrationListItem extends BaseListItem @@ -67,8 +68,7 @@ class MigrationListItem extends BaseListItem if ($this->item->getDescription()) { $caption->addHtml(Text::create($this->item->getDescription())); } else { - $caption->getAttributes()->add('class', 'empty-state'); - $caption->addHtml(Text::create($this->translate('No description provided.'))); + $caption->addHtml(new EmptyState(Text::create($this->translate('No description provided.')))); } } diff --git a/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php b/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php index f4137e39c..a45288808 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php @@ -17,6 +17,7 @@ use ipl\Html\Text; use ipl\I18n\Translation; use ipl\Web\Common\BaseListItem; use ipl\Web\Url; +use ipl\Web\Widget\EmptyState; use ipl\Web\Widget\Icon; use ipl\Web\Widget\Link; use LogicException; @@ -74,8 +75,7 @@ class MigrationListItemMinimal extends BaseListItem if ($migration->getDescription()) { $caption->addHtml(Text::create($migration->getDescription())); } else { - $caption->getAttributes()->add('class', 'empty-state'); - $caption->addHtml(Text::create($this->translate('No description provided.'))); + $caption->addHtml(new EmptyState(Text::create($this->translate('No description provided.')))); } $scriptPath = $migration->getScriptPath(); diff --git a/public/css/icinga/pending-migration.less b/public/css/icinga/pending-migration.less index 46ba09b11..96f693bd4 100644 --- a/public/css/icinga/pending-migration.less +++ b/public/css/icinga/pending-migration.less @@ -25,10 +25,6 @@ color: @state-critical; } - .caption.empty-state { - color: @gray-semilight; - } - span.version { color: @text-color; } From a167b6d21af091c55bf85045f3b82f9bc1b51b7c Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 13 Sep 2023 10:37:14 +0200 Subject: [PATCH 25/48] Rename migration list item classes --- .../Widget/ItemList/MigrationFileListItem.php | 92 +++++++++++ .../Web/Widget/ItemList/MigrationList.php | 8 +- .../Web/Widget/ItemList/MigrationListItem.php | 129 +++++++++++---- .../ItemList/MigrationListItemMinimal.php | 151 ------------------ 4 files changed, 190 insertions(+), 190 deletions(-) create mode 100644 library/Icinga/Web/Widget/ItemList/MigrationFileListItem.php delete mode 100644 library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php diff --git a/library/Icinga/Web/Widget/ItemList/MigrationFileListItem.php b/library/Icinga/Web/Widget/ItemList/MigrationFileListItem.php new file mode 100644 index 000000000..9ce63a881 --- /dev/null +++ b/library/Icinga/Web/Widget/ItemList/MigrationFileListItem.php @@ -0,0 +1,92 @@ +item->getLastState()) { + $visual->getAttributes()->add('class', 'upgrade-failed'); + $visual->addHtml(new Icon('circle-xmark')); + } + } + + protected function assembleTitle(BaseHtmlElement $title): void + { + $scriptPath = $this->item->getScriptPath(); + /** @var string $parentDirs */ + $parentDirs = substr($scriptPath, (int) strpos($scriptPath, 'schema')); + $parentDirs = substr($parentDirs, 0, strrpos($parentDirs, '/') + 1); + + $title->addHtml( + new HtmlElement('span', null, Text::create($parentDirs)), + new HtmlElement( + 'span', + Attributes::create(['class' => 'version']), + Text::create($this->item->getVersion() . '.sql') + ) + ); + + if ($this->item->getLastState()) { + $title->addHtml( + new HtmlElement( + 'span', + Attributes::create(['class' => 'upgrade-failed']), + Text::create($this->translate('Upgrade failed')) + ) + ); + } + } + + protected function assembleHeader(BaseHtmlElement $header): void + { + $header->addHtml($this->createTitle()); + } + + protected function assembleCaption(BaseHtmlElement $caption): void + { + if ($this->item->getDescription()) { + $caption->addHtml(Text::create($this->item->getDescription())); + } else { + $caption->addHtml(new EmptyState(Text::create($this->translate('No description provided.')))); + } + } + + protected function assembleFooter(BaseHtmlElement $footer): void + { + if ($this->item->getLastState()) { + $footer->addHtml( + new HtmlElement( + 'section', + Attributes::create(['class' => 'caption']), + new HtmlElement('pre', null, new HtmlString(Html::escape($this->item->getLastState()))) + ) + ); + } + } + + protected function assembleMain(BaseHtmlElement $main): void + { + $main->addHtml($this->createHeader(), $this->createCaption()); + } +} diff --git a/library/Icinga/Web/Widget/ItemList/MigrationList.php b/library/Icinga/Web/Widget/ItemList/MigrationList.php index d1836cd98..b8a153b31 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationList.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationList.php @@ -69,10 +69,10 @@ class MigrationList extends BaseItemList protected function getItemClass(): string { if ($this->isMinimal()) { - return MigrationListItemMinimal::class; + return MigrationListItem::class; } - return MigrationListItem::class; + return MigrationFileListItem::class; } protected function assemble(): void @@ -84,9 +84,9 @@ class MigrationList extends BaseItemList /** @var MigrationHook $data */ foreach ($this->data as $data) { - /** @var MigrationListItem|MigrationListItemMinimal $item */ + /** @var MigrationFileListItem|MigrationListItem $item */ $item = new $itemClass($data, $this); - if ($item instanceof MigrationListItemMinimal && $this->migrationForm) { + if ($item instanceof MigrationListItem && $this->migrationForm) { $migrateButton = $this->migrationForm->createElement( 'submit', sprintf('migrate-%s', $data->getModuleName()), diff --git a/library/Icinga/Web/Widget/ItemList/MigrationListItem.php b/library/Icinga/Web/Widget/ItemList/MigrationListItem.php index 89ceade82..d9010db22 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationListItem.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationListItem.php @@ -5,81 +5,131 @@ namespace Icinga\Web\Widget\ItemList; use Icinga\Application\Hook\Common\DbMigration; +use Icinga\Application\Hook\MigrationHook; use ipl\Html\Attributes; use ipl\Html\BaseHtmlElement; +use ipl\Html\Contract\FormElement; +use ipl\Html\FormattedString; use ipl\Html\Html; use ipl\Html\HtmlElement; use ipl\Html\HtmlString; use ipl\Html\Text; use ipl\I18n\Translation; use ipl\Web\Common\BaseListItem; +use ipl\Web\Url; use ipl\Web\Widget\EmptyState; use ipl\Web\Widget\Icon; +use ipl\Web\Widget\Link; +use LogicException; class MigrationListItem extends BaseListItem { use Translation; - /** @var DbMigration Just for type hint */ + /** @var ?FormElement */ + protected $migrateButton; + + /** @var MigrationHook Just for type hint */ protected $item; - protected function assembleVisual(BaseHtmlElement $visual): void + /** + * Set a migration form of this list item + * + * @param FormElement $migrateButton + * + * @return $this + */ + public function setMigrateButton(FormElement $migrateButton): self { - if ($this->item->getLastState()) { - $visual->getAttributes()->add('class', 'upgrade-failed'); - $visual->addHtml(new Icon('circle-xmark')); - } + $this->migrateButton = $migrateButton; + + return $this; } protected function assembleTitle(BaseHtmlElement $title): void { - $scriptPath = $this->item->getScriptPath(); - /** @var string $parentDirs */ - $parentDirs = substr($scriptPath, (int) strpos($scriptPath, 'schema')); - $parentDirs = substr($parentDirs, 0, strrpos($parentDirs, '/') + 1); - $title->addHtml( - new HtmlElement('span', null, Text::create($parentDirs)), - new HtmlElement( - 'span', - Attributes::create(['class' => 'version']), - Text::create($this->item->getVersion() . '.sql') + FormattedString::create( + t('%s ', ''), + HtmlElement::create('span', ['class' => 'subject'], $this->item->getName()) ) ); + } - if ($this->item->getLastState()) { + protected function assembleHeader(BaseHtmlElement $header): void + { + if ($this->migrateButton === null) { + throw new LogicException('Please set the migrate submit button beforehand'); + } + + $header->addHtml($this->createTitle()); + $header->addHtml($this->migrateButton); + } + + protected function assembleCaption(BaseHtmlElement $caption): void + { + $migrations = $this->item->getMigrations(); + /** @var DbMigration $migration */ + $migration = array_shift($migrations); + if ($migration->getLastState()) { + if ($migration->getDescription()) { + $caption->addHtml(Text::create($migration->getDescription())); + } else { + $caption->addHtml(new EmptyState(Text::create($this->translate('No description provided.')))); + } + + $scriptPath = $migration->getScriptPath(); + /** @var string $parentDirs */ + $parentDirs = substr($scriptPath, (int) strpos($scriptPath, 'schema')); + $parentDirs = substr($parentDirs, 0, strrpos($parentDirs, '/') + 1); + + $title = new HtmlElement('div', Attributes::create(['class' => 'title'])); $title->addHtml( + new HtmlElement('span', null, Text::create($parentDirs)), + new HtmlElement( + 'span', + Attributes::create(['class' => 'version']), + Text::create($migration->getVersion() . '.sql') + ), new HtmlElement( 'span', Attributes::create(['class' => 'upgrade-failed']), Text::create($this->translate('Upgrade failed')) ) ); - } - } - protected function assembleHeader(BaseHtmlElement $header): void - { - $header->addHtml($this->createTitle()); - } + $error = new HtmlElement('div', Attributes::create([ + 'class' => 'collapsible', + 'data-visible-height' => '58', + ])); + $error->addHtml(new HtmlElement('pre', null, new HtmlString(Html::escape($migration->getLastState())))); - protected function assembleCaption(BaseHtmlElement $caption): void - { - if ($this->item->getDescription()) { - $caption->addHtml(Text::create($this->item->getDescription())); - } else { - $caption->addHtml(new EmptyState(Text::create($this->translate('No description provided.')))); + $errorSection = new HtmlElement('div', Attributes::create(['class' => 'errors-section',])); + $errorSection->addHtml( + new HtmlElement('header', null, new Icon('circle-xmark', ['class' => 'status-icon']), $title), + $caption, + $error + ); + + $caption->prependWrapper($errorSection); } } protected function assembleFooter(BaseHtmlElement $footer): void { - if ($this->item->getLastState()) { + $footer->addHtml((new MigrationList($this->item->getLatestMigrations(3)))->setMinimal(false)); + if ($this->item->count() > 3) { $footer->addHtml( - new HtmlElement( - 'section', - Attributes::create(['class' => 'caption']), - new HtmlElement('pre', null, new HtmlString(Html::escape($this->item->getLastState()))) + new Link( + sprintf($this->translate('Show all %d migrations'), $this->item->count()), + Url::fromPath( + 'migrations/migration', + [MigrationHook::MIGRATION_PARAM => $this->item->getModuleName()] + ), + [ + 'data-base-target' => '_next', + 'class' => 'show-more' + ] ) ); } @@ -87,6 +137,15 @@ class MigrationListItem extends BaseListItem protected function assembleMain(BaseHtmlElement $main): void { - $main->addHtml($this->createHeader(), $this->createCaption()); + $main->addHtml($this->createHeader()); + $caption = $this->createCaption(); + if (! $caption->isEmpty()) { + $main->addHtml($caption); + } + + $footer = $this->createFooter(); + if ($footer) { + $main->addHtml($footer); + } } } diff --git a/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php b/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php deleted file mode 100644 index a45288808..000000000 --- a/library/Icinga/Web/Widget/ItemList/MigrationListItemMinimal.php +++ /dev/null @@ -1,151 +0,0 @@ -migrateButton = $migrateButton; - - return $this; - } - - protected function assembleTitle(BaseHtmlElement $title): void - { - $title->addHtml( - FormattedString::create( - t('%s ', ''), - HtmlElement::create('span', ['class' => 'subject'], $this->item->getName()) - ) - ); - } - - protected function assembleHeader(BaseHtmlElement $header): void - { - if ($this->migrateButton === null) { - throw new LogicException('Please set the migrate submit button beforehand'); - } - - $header->addHtml($this->createTitle()); - $header->addHtml($this->migrateButton); - } - - protected function assembleCaption(BaseHtmlElement $caption): void - { - $migrations = $this->item->getMigrations(); - /** @var DbMigration $migration */ - $migration = array_shift($migrations); - if ($migration->getLastState()) { - if ($migration->getDescription()) { - $caption->addHtml(Text::create($migration->getDescription())); - } else { - $caption->addHtml(new EmptyState(Text::create($this->translate('No description provided.')))); - } - - $scriptPath = $migration->getScriptPath(); - /** @var string $parentDirs */ - $parentDirs = substr($scriptPath, (int) strpos($scriptPath, 'schema')); - $parentDirs = substr($parentDirs, 0, strrpos($parentDirs, '/') + 1); - - $title = new HtmlElement('div', Attributes::create(['class' => 'title'])); - $title->addHtml( - new HtmlElement('span', null, Text::create($parentDirs)), - new HtmlElement( - 'span', - Attributes::create(['class' => 'version']), - Text::create($migration->getVersion() . '.sql') - ), - new HtmlElement( - 'span', - Attributes::create(['class' => 'upgrade-failed']), - Text::create($this->translate('Upgrade failed')) - ) - ); - - $error = new HtmlElement('div', Attributes::create([ - 'class' => 'collapsible', - 'data-visible-height' => '58', - ])); - $error->addHtml(new HtmlElement('pre', null, new HtmlString(Html::escape($migration->getLastState())))); - - $errorSection = new HtmlElement('div', Attributes::create(['class' => 'errors-section',])); - $errorSection->addHtml( - new HtmlElement('header', null, new Icon('circle-xmark', ['class' => 'status-icon']), $title), - $caption, - $error - ); - - $caption->prependWrapper($errorSection); - } - } - - protected function assembleFooter(BaseHtmlElement $footer): void - { - $footer->addHtml((new MigrationList($this->item->getLatestMigrations(3)))->setMinimal(false)); - if ($this->item->count() > 3) { - $footer->addHtml( - new Link( - sprintf($this->translate('Show all %d migrations'), $this->item->count()), - Url::fromPath( - 'migrations/migration', - [MigrationHook::MIGRATION_PARAM => $this->item->getModuleName()] - ), - [ - 'data-base-target' => '_next', - 'class' => 'show-more' - ] - ) - ); - } - } - - protected function assembleMain(BaseHtmlElement $main): void - { - $main->addHtml($this->createHeader()); - $caption = $this->createCaption(); - if (! $caption->isEmpty()) { - $main->addHtml($caption); - } - - $footer = $this->createFooter(); - if ($footer) { - $main->addHtml($footer); - } - } -} From 73b104181613eb805972ebc7fe2aac02f7f8bb1c Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 13 Sep 2023 14:59:22 +0200 Subject: [PATCH 26/48] Fix phpstan claims & php code sniffer errors --- application/controllers/MigrationsController.php | 4 ++-- library/Icinga/Application/Hook/MigrationHook.php | 13 +++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/application/controllers/MigrationsController.php b/application/controllers/MigrationsController.php index ca2c83f4c..1a6fc6a27 100644 --- a/application/controllers/MigrationsController.php +++ b/application/controllers/MigrationsController.php @@ -203,8 +203,8 @@ class MigrationsController extends CompatController } else { Notification::error( $this->translate( - 'Applied migrations successfully. Though, one or more migration hooks failed to run.' - . ' See logs for details' + 'Applied migrations successfully. Though, one or more migration hooks' + . ' failed to run. See logs for details' ) ); } diff --git a/library/Icinga/Application/Hook/MigrationHook.php b/library/Icinga/Application/Hook/MigrationHook.php index 9e31f3421..1811b9839 100644 --- a/library/Icinga/Application/Hook/MigrationHook.php +++ b/library/Icinga/Application/Hook/MigrationHook.php @@ -86,7 +86,8 @@ abstract class MigrationHook implements Countable { $pdoStmt = $conn->prepexec( sprintf( - "SELECT %s AS column_type, %s AS column_length FROM information_schema.columns WHERE table_name = ? AND column_name = ?", + 'SELECT %s AS column_type, %s AS column_length FROM information_schema.columns' + . ' WHERE table_name = ? AND column_name = ?', $conn->getAdapter() instanceof Pgsql ? 'udt_name' : 'column_type', $conn->getAdapter() instanceof Pgsql ? 'character_maximum_length' : 'NULL' ), @@ -140,7 +141,7 @@ abstract class MigrationHook implements Countable $this->load(); } - return $this->migrations; + return $this->migrations ?? []; } /** @@ -289,8 +290,12 @@ abstract class MigrationHook implements Countable } } - // Sort all the migrations by their version numbers in ascending order. - uksort($this->migrations, 'version_compare'); + if ($this->migrations) { + // Sort all the migrations by their version numbers in ascending order. + uksort($this->migrations, function ($a, $b) { + return version_compare($a, $b); + }); + } } /** From ac24c6d34b1b973b4d26aebf2e2d6938d2941ad3 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 13 Sep 2023 15:15:08 +0200 Subject: [PATCH 27/48] Don't traverse schema query if the last successfully migrated version is found --- library/Icinga/Application/ProvidedHook/DbMigration.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/Icinga/Application/ProvidedHook/DbMigration.php b/library/Icinga/Application/ProvidedHook/DbMigration.php index 2f6ecc1be..5411ae157 100644 --- a/library/Icinga/Application/ProvidedHook/DbMigration.php +++ b/library/Icinga/Application/ProvidedHook/DbMigration.php @@ -36,6 +36,8 @@ class DbMigration extends MigrationHook foreach ($schemaQuery as $schema) { if ($schema->success) { $this->version = $schema->version; + + break; } } From 80ac314d8bd5fdeb1d265f36595238b3a5852a1d Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 13 Sep 2023 15:30:13 +0200 Subject: [PATCH 28/48] Schema: Update timestamp & set success of existing schema version entry --- schema/mysql-upgrades/2.12.0.sql | 2 ++ schema/pgsql-upgrades/2.12.0.sql | 2 ++ 2 files changed, 4 insertions(+) diff --git a/schema/mysql-upgrades/2.12.0.sql b/schema/mysql-upgrades/2.12.0.sql index 9aace2856..d72a016c9 100644 --- a/schema/mysql-upgrades/2.12.0.sql +++ b/schema/mysql-upgrades/2.12.0.sql @@ -6,6 +6,8 @@ ALTER TABLE icingaweb_schema DROP CONSTRAINT IF EXISTS idx_icingaweb_schema_version, ADD CONSTRAINT idx_icingaweb_schema_version UNIQUE (version); +UPDATE icingaweb_schema SET timestamp = timestamp * 1000, success = 'y'; + INSERT INTO icingaweb_schema (version, timestamp, success, reason) VALUES('2.12.0', UNIX_TIMESTAMP() * 1000, 'y', NULL) ON DUPLICATE KEY UPDATE timestamp = VALUES(timestamp), success = VALUES(success), reason = VALUES(reason); diff --git a/schema/pgsql-upgrades/2.12.0.sql b/schema/pgsql-upgrades/2.12.0.sql index 733d8f547..706a28c69 100644 --- a/schema/pgsql-upgrades/2.12.0.sql +++ b/schema/pgsql-upgrades/2.12.0.sql @@ -8,6 +8,8 @@ ALTER TABLE icingaweb_schema DROP CONSTRAINT IF EXISTS idx_icingaweb_schema_version, ADD CONSTRAINT idx_icingaweb_schema_version UNIQUE (version); +UPDATE icingaweb_schema SET timestamp = timestamp * 1000, success = 'y'; + INSERT INTO icingaweb_schema (version, timestamp, success, reason) VALUES('2.12.0', EXTRACT(EPOCH FROM now()) * 1000, 'y', NULL) ON CONFLICT ON CONSTRAINT idx_icingaweb_schema_version DO UPDATE SET timestamp = EXCLUDED.timestamp, success = EXCLUDED.success, reason = EXCLUDED.reason; From ce2073d7bfbda771fc75adaabd85b7297a8b3477 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 13 Sep 2023 15:42:52 +0200 Subject: [PATCH 29/48] Add `2.12` database upgrade docs --- doc/80-Upgrading.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/doc/80-Upgrading.md b/doc/80-Upgrading.md index ee3602d82..c6f4b7b57 100644 --- a/doc/80-Upgrading.md +++ b/doc/80-Upgrading.md @@ -3,6 +3,17 @@ Specific version upgrades are described below. Please note that upgrades are incremental. An upgrade from v2.6 to v2.8 requires to follow the instructions for v2.7 too. +## Upgrading to Icinga Web 2.12.0 + +**Database Schema** + +With the latest Icinga Web versions, you no longer need to manually import sql upgrade scripts. Icinga Web `>= 2.12` +offers you the possibility to perform such migrations in an easy way. You can find and apply all pending migrations +of your Icinga Web environment in the menu at `System -> Migrations`. + +You can still apply the `2.12.0.sql` upgrade script manually, depending on your database vendor. +For package installations you can find this file in `/usr/share/icingaweb2/schema/*-upgrades/`. + ## Upgrading to Icinga Web 2.11.x **General** From 12bc95099ee06e0f59ad440b3e0db519e55e9467 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Thu, 14 Sep 2023 17:09:11 +0200 Subject: [PATCH 30/48] Don't raise unhandled exceptions in menu context --- library/Icinga/Web/Navigation/ConfigMenu.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/library/Icinga/Web/Navigation/ConfigMenu.php b/library/Icinga/Web/Navigation/ConfigMenu.php index 333144a92..583bf42bb 100644 --- a/library/Icinga/Web/Navigation/ConfigMenu.php +++ b/library/Icinga/Web/Navigation/ConfigMenu.php @@ -16,6 +16,7 @@ use ipl\Html\Text; use ipl\Web\Url; use ipl\Web\Widget\Icon; use ipl\Web\Widget\StateBadge; +use Throwable; class ConfigMenu extends BaseHtmlElement { @@ -230,8 +231,13 @@ class ConfigMenu extends BaseHtmlElement protected function createMigrationBadge(): ?StateBadge { - $mm = MigrationManager::instance(); - $count = $mm->count(); + try { + $mm = MigrationManager::instance(); + $count = $mm->count(); + } catch (Throwable $e) { + Logger::error('Failed to load pending migrations: %s', $e); + $count = 0; + } $stateBadge = null; if ($count > 0) { From 2657f032dc1e403a274503f48835bd14828b18de Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Thu, 14 Sep 2023 17:17:16 +0200 Subject: [PATCH 31/48] Allow to automatically fix missing grants & elevalte database users Co-authored-by: Johannes Meyer --- .../controllers/MigrationsController.php | 22 ++- application/forms/MigrationForm.php | 102 +++++++++++ .../Application/Hook/Common/DbMigration.php | 17 +- .../Icinga/Application/Hook/MigrationHook.php | 26 +-- .../Icinga/Application/MigrationManager.php | 169 +++++++++++++++++- .../Application/ProvidedHook/DbMigration.php | 10 +- public/css/icinga/pending-migration.less | 23 ++- 7 files changed, 327 insertions(+), 42 deletions(-) diff --git a/application/controllers/MigrationsController.php b/application/controllers/MigrationsController.php index 1a6fc6a27..e1bcd5c4d 100644 --- a/application/controllers/MigrationsController.php +++ b/application/controllers/MigrationsController.php @@ -4,8 +4,11 @@ namespace Icinga\Controllers; +use Exception; use Icinga\Application\Hook\MigrationHook; +use Icinga\Application\Icinga; use Icinga\Application\MigrationManager; +use Icinga\Common\Database; use Icinga\Exception\MissingParameterException; use Icinga\Forms\MigrationForm; use Icinga\Web\Notification; @@ -19,6 +22,13 @@ use ipl\Web\Widget\ActionLink; class MigrationsController extends CompatController { + use Database; + + public function init() + { + Icinga::app()->getModuleManager()->loadModule('setup'); + } + public function indexAction(): void { $mm = MigrationManager::instance(); @@ -44,6 +54,8 @@ class MigrationsController extends CompatController } $migrateListForm = new MigrationForm(); + $migrateListForm->setRenderDatabaseUserChange(! $mm->validateDatabasePrivileges()); + $migrateGlobalForm = new MigrationForm(); $migrateGlobalForm->getAttributes()->set('name', sprintf('migrate-%s', MigrationHook::ALL_MIGRATIONS)); @@ -193,12 +205,18 @@ class MigrationsController extends CompatController $form->on(MigrationForm::ON_SUCCESS, function (MigrationForm $form) { $mm = MigrationManager::instance(); + /** @var array $elevatedPrivileges */ + $elevatedPrivileges = $form->getValue('database_setup'); + if ($elevatedPrivileges !== null && $elevatedPrivileges['grant_privileges'] === 'y') { + $mm->fixIcingaWebMysqlGrants($this->getDb(), $elevatedPrivileges); + } + $pressedButton = $form->getPressedSubmitElement(); if ($pressedButton) { $name = substr($pressedButton->getName(), 8); switch ($name) { case MigrationHook::ALL_MIGRATIONS: - if ($mm->applyAll()) { + if ($mm->applyAll($elevatedPrivileges)) { Notification::success($this->translate('Applied all migrations successfully')); } else { Notification::error( @@ -211,7 +229,7 @@ class MigrationsController extends CompatController break; default: $migration = $mm->getMigration($name); - if ($mm->apply($migration)) { + if ($mm->apply($migration, $elevatedPrivileges)) { Notification::success($this->translate('Applied pending migrations successfully')); } else { Notification::error( diff --git a/application/forms/MigrationForm.php b/application/forms/MigrationForm.php index 940479153..120c84684 100644 --- a/application/forms/MigrationForm.php +++ b/application/forms/MigrationForm.php @@ -4,20 +4,34 @@ namespace Icinga\Forms; +use Icinga\Application\MigrationManager; +use ipl\Html\Attributes; use ipl\Html\Form; +use ipl\Html\FormElement\CheckboxElement; +use ipl\Html\FormElement\FieldsetElement; +use ipl\Html\HtmlElement; +use ipl\Html\Text; +use ipl\I18n\Translation; +use ipl\Validator\CallbackValidator; use ipl\Web\Common\CsrfCounterMeasure; use ipl\Web\Common\FormUid; +use ipl\Web\FormDecorator\IcingaFormDecorator; +use PDOException; class MigrationForm extends Form { use CsrfCounterMeasure; use FormUid; + use Translation; protected $defaultAttributes = [ 'class' => ['icinga-form', 'migration-form', 'icinga-controls'], 'name' => 'migration-form' ]; + /** @var bool Whether to allow changing the current database user and password */ + protected $renderDatabaseUserChange = false; + public function hasBeenSubmitted(): bool { if (! $this->hasBeenSent()) { @@ -25,11 +39,99 @@ class MigrationForm extends Form } $pressedButton = $this->getPressedSubmitElement(); + return $pressedButton && strpos($pressedButton->getName(), 'migrate-') !== false; } + public function setRenderDatabaseUserChange(bool $value = true): self + { + $this->renderDatabaseUserChange = $value; + + return $this; + } + protected function assemble(): void { $this->addHtml($this->createUidElement()); + + if ($this->renderDatabaseUserChange) { + $mm = MigrationManager::instance(); + $newDbSetup = new FieldsetElement('database_setup', ['required' => true]); + $newDbSetup + ->setDefaultElementDecorator(new IcingaFormDecorator()) + ->addElement('text', 'username', [ + 'required' => true, + 'label' => $this->translate('Username'), + 'description' => $this->translate( + 'A user which is able to create and/or alter the database schema.' + ) + ]) + ->addElement('password', 'password', [ + 'required' => true, + 'autocomplete' => 'new-password', + 'label' => $this->translate('Password'), + 'description' => $this->translate('The password for the database user defined above.'), + 'validators' => [ + new CallbackValidator(function ($_, CallbackValidator $validator) use ($mm, $newDbSetup): bool { + /** @var array $values */ + $values = $this->getValue('database_setup'); + /** @var CheckboxElement $checkBox */ + $checkBox = $newDbSetup->getElement('grant_privileges'); + $canIssueGrants = $checkBox->isChecked(); + $elevationConfig = [ + 'username' => $values['username'], + 'password' => $values['password'] + ]; + + try { + if (! $mm->validateDatabasePrivileges($elevationConfig, $canIssueGrants)) { + $validator->addMessage(sprintf( + $this->translate( + 'The provided credentials cannot be used to execute "%s" SQL commands' + . ' and/or grant the missing privileges to other users.' + ), + implode(' ,', $mm->getRequiredDatabasePrivileges()) + )); + + return false; + } + } catch (PDOException $e) { + $validator->addMessage($e->getMessage()); + + return false; + } + + return true; + }) + ] + ]) + ->addElement('checkbox', 'grant_privileges', [ + 'required' => false, + 'label' => $this->translate('Grant Missing Privileges'), + 'description' => $this->translate( + 'Allows to automatically grant the required privileges to the database user specified' + . ' in the respective resource config. If you do not want to provide additional credentials' + . ' each time, you can enable this and Icinga Web will grant the active database user the' + . ' missing privileges.' + ) + ]); + + $this->addHtml( + new HtmlElement( + 'div', + Attributes::create(['class' => 'change-database-user-description']), + new HtmlElement('span', null, Text::create(sprintf( + $this->translate( + 'It seems that the currently used database user does not have the required privileges to' + . ' execute the %s SQL commands. Please provide an alternative user' + . ' that has the appropriate credentials to resolve this issue.' + ), + implode(', ', $mm->getRequiredDatabasePrivileges()) + ))) + ) + ); + + $this->addElement($newDbSetup); + } } } diff --git a/library/Icinga/Application/Hook/Common/DbMigration.php b/library/Icinga/Application/Hook/Common/DbMigration.php index a115e10d1..ae79da785 100644 --- a/library/Icinga/Application/Hook/Common/DbMigration.php +++ b/library/Icinga/Application/Hook/Common/DbMigration.php @@ -27,8 +27,7 @@ class DbMigration public function __construct(string $version, string $scriptPath) { $this->scriptPath = $scriptPath; - - $this->setVersion($version); + $this->version = $version; } /** @@ -41,20 +40,6 @@ class DbMigration return $this->version; } - /** - * Set the sql script version the queries are loaded from - * - * @param string $version - * - * @return $this - */ - public function setVersion(string $version): self - { - $this->version = $version; - - return $this; - } - /** * Get upgrade script relative path name * diff --git a/library/Icinga/Application/Hook/MigrationHook.php b/library/Icinga/Application/Hook/MigrationHook.php index 1811b9839..4b8c632d0 100644 --- a/library/Icinga/Application/Hook/MigrationHook.php +++ b/library/Icinga/Application/Hook/MigrationHook.php @@ -14,13 +14,16 @@ use Icinga\Application\Icinga; use Icinga\Application\Logger; use Icinga\Application\Modules\Module; use Icinga\Model\Schema; +use Icinga\Module\Setup\Utils\DbTool; use Icinga\Web\Session; use ipl\I18n\Translation; use ipl\Orm\Query; use ipl\Sql\Adapter\Pgsql; use ipl\Sql\Connection; use ipl\Stdlib\Filter; +use ipl\Stdlib\Str; use PDO; +use PDOException; use SplFileInfo; use stdClass; @@ -128,6 +131,13 @@ abstract class MigrationHook implements Countable */ abstract public function getVersion(): string; + /** + * Get a database connection + * + * @return Connection + */ + abstract public function getDb(): Connection; + /** * Get all the pending migrations of this hook * @@ -166,9 +176,12 @@ abstract class MigrationHook implements Countable * * @return bool Whether the migration(s) have been successfully applied */ - public function run(): bool + final public function run(Connection $conn = null): bool { - $conn = $this->getDb(); + if (! $conn) { + $conn = $this->getDb(); + } + foreach ($this->getMigrations() as $migration) { try { $migration->apply($conn); @@ -176,7 +189,7 @@ abstract class MigrationHook implements Countable $this->version = $migration->getVersion(); unset($this->migrations[$migration->getVersion()]); - Logger::error( + Logger::info( "Applied %s pending migration version %s successfully", $this->getName(), $migration->getVersion() @@ -243,13 +256,6 @@ abstract class MigrationHook implements Countable return count($this->getMigrations()); } - /** - * Get a database connection - * - * @return Connection - */ - abstract protected function getDb(): Connection; - /** * Get a schema version query * diff --git a/library/Icinga/Application/MigrationManager.php b/library/Icinga/Application/MigrationManager.php index b1065261e..6151a84c2 100644 --- a/library/Icinga/Application/MigrationManager.php +++ b/library/Icinga/Application/MigrationManager.php @@ -8,11 +8,14 @@ use Countable; use Generator; use Icinga\Application\Hook\MigrationHook; use Icinga\Exception\NotFoundError; +use Icinga\Module\Setup\Utils\DbTool; +use Icinga\Module\Setup\WebWizard; use ipl\I18n\Translation; +use ipl\Sql; +use ReflectionClass; /** - * Migration manager encapsulates PHP code and DB migrations and manages all pending migrations in a - * structured way. + * Migration manager allows you to manage all pending migrations in a structured way. */ final class MigrationManager implements Countable { @@ -82,7 +85,7 @@ final class MigrationManager implements Countable * * @return MigrationHook * - * @throws NotFoundError When there are no pending PHP code migrations matching the given module name + * @throws NotFoundError When there are no pending migrations matching the given module name */ public function getMigration(string $module): MigrationHook { @@ -124,10 +127,11 @@ final class MigrationManager implements Countable * Apply the given migration hook * * @param MigrationHook $hook + * @param ?array $elevateConfig * * @return bool */ - public function apply(MigrationHook $hook): bool + public function apply(MigrationHook $hook, array $elevateConfig = null): bool { if ($hook->isModule() && $this->hasMigrations(MigrationHook::DEFAULT_MODULE)) { Logger::error( @@ -137,7 +141,12 @@ final class MigrationManager implements Countable return false; } - if ($hook->run()) { + $conn = $hook->getDb(); + if ($elevateConfig && ! $this->checkRequiredPrivileges($conn)) { + $conn = $this->elevateDatabaseConnection($conn, $elevateConfig); + } + + if ($hook->run($conn)) { unset($this->pendingMigrations[$hook->getModuleName()]); Logger::info('Applied pending %s migrations successfully', $hook->getName()); @@ -151,21 +160,23 @@ final class MigrationManager implements Countable /** * Apply all pending modules/framework migrations * + * @param ?array $elevateConfig + * * @return bool */ - public function applyAll(): bool + public function applyAll(array $elevateConfig = null): bool { $default = MigrationHook::DEFAULT_MODULE; if ($this->hasMigrations($default)) { $migration = $this->getMigration($default); - if (! $this->apply($migration)) { + if (! $this->apply($migration, $elevateConfig)) { return false; } } $succeeded = true; foreach ($this->getPendingMigrations() as $migration) { - if (! $this->apply($migration) && $succeeded) { + if (! $this->apply($migration, $elevateConfig) && $succeeded) { $succeeded = false; } } @@ -189,6 +200,99 @@ final class MigrationManager implements Countable } } + /** + * Get the required database privileges for database migrations + * + * @return string[] + */ + public function getRequiredDatabasePrivileges(): array + { + return ['CREATE','SELECT','INSERT','UPDATE','DELETE','DROP','ALTER','CREATE VIEW','INDEX','EXECUTE']; + } + + /** + * Verify whether all database users of all pending migrations do have the required SQL privileges + * + * @param ?array $elevateConfig + * @param bool $canIssueGrant + * + * @return bool + */ + public function validateDatabasePrivileges(array $elevateConfig = null, bool $canIssueGrant = false): bool + { + if (! $this->hasPendingMigrations()) { + return true; + } + + foreach ($this->getPendingMigrations() as $migration) { + if (! $this->checkRequiredPrivileges($migration->getDb(), $elevateConfig, $canIssueGrant)) { + return false; + } + } + + return true; + } + + /** + * Check if there are missing grants for the Icinga Web database and fix them + * + * This fixes the following problems on existing installations: + * - Setups made by the wizard have no access to `icingaweb_schema` + * - Setups made by the wizard have no DDL grants + * - Setups done manually using the advanced documentation chapter have no DDL grants + * + * @param Sql\Connection $db + * @param array $elevateConfig + */ + public function fixIcingaWebMysqlGrants(Sql\Connection $db, array $elevateConfig): void + { + $wizardProperties = (new ReflectionClass(WebWizard::class)) + ->getDefaultProperties(); + /** @var array $privileges */ + $privileges = $wizardProperties['databaseUsagePrivileges']; + /** @var array $tables */ + $tables = $wizardProperties['databaseTables']; + + $actualUsername = $db->getConfig()->username; + $db = $this->elevateDatabaseConnection($db, $elevateConfig); + $tool = $this->createDbTool($db); + $tool->connectToDb(); + + if ($tool->checkPrivileges(['SELECT'], [], $actualUsername)) { + // Checks only database level grants. If this succeeds, the grants were issued manually. + if (! $tool->checkPrivileges($privileges, [], $actualUsername) && $tool->isGrantable($privileges)) { + // Any missing grant is now granted on database level as well, not to mix things up + $tool->grantPrivileges($privileges, [], $actualUsername); + } + } elseif (! $tool->checkPrivileges($privileges, $tables, $actualUsername) && $tool->isGrantable($privileges)) { + // The above ensures that if this fails, we can safely apply table level grants, as it's + // very likely that the existing grants were issued by the setup wizard + $tool->grantPrivileges($privileges, $tables, $actualUsername); + } + } + + /** + * Create and return a DbTool instance + * + * @param Sql\Connection $db + * + * @return DbTool + */ + private function createDbTool(Sql\Connection $db): DbTool + { + $config = $db->getConfig(); + + return new DbTool(array_merge([ + 'db' => $config->db, + 'host' => $config->host, + 'port' => $config->port, + 'dbname' => $config->dbname, + 'username' => $config->username, + 'password' => $config->password, + 'charset' => $config->charset + ], $db->getAdapter()->getOptions($config))); + } + protected function load(): void { $this->pendingMigrations = []; @@ -205,6 +309,55 @@ final class MigrationManager implements Countable ksort($this->pendingMigrations); } + /** + * Check the required SQL privileges of the given connection + * + * @param Sql\Connection $conn + * @param ?array $elevateConfig + * @param bool $canIssueGrants + * + * @return bool + */ + protected function checkRequiredPrivileges( + Sql\Connection $conn, + array $elevateConfig = null, + bool $canIssueGrants = false + ): bool { + if ($elevateConfig) { + $conn = $this->elevateDatabaseConnection($conn, $elevateConfig); + } + + $dbTool = $this->createDbTool($conn); + $dbTool->connectToDb(); + if (! $dbTool->checkPrivileges($this->getRequiredDatabasePrivileges())) { + return false; + } + + if ($canIssueGrants && ! $dbTool->isGrantable($this->getRequiredDatabasePrivileges())) { + return false; + } + + return true; + } + + /** + * Override the database config of the given connection by the specified new config + * + * Overrides only the username and password of existing database connection. + * + * @param Sql\Connection $conn + * @param array $elevateConfig + * @return Sql\Connection + */ + protected function elevateDatabaseConnection(Sql\Connection $conn, array $elevateConfig): Sql\Connection + { + $config = clone $conn->getConfig(); + $config->username = $elevateConfig['username']; + $config->password = $elevateConfig['password']; + + return new Sql\Connection($config); + } + /** * Get all pending migrations as an array * diff --git a/library/Icinga/Application/ProvidedHook/DbMigration.php b/library/Icinga/Application/ProvidedHook/DbMigration.php index 5411ae157..37d299535 100644 --- a/library/Icinga/Application/ProvidedHook/DbMigration.php +++ b/library/Icinga/Application/ProvidedHook/DbMigration.php @@ -8,10 +8,18 @@ use Icinga\Application\Hook\MigrationHook; use Icinga\Common\Database; use Icinga\Model\Schema; use ipl\Orm\Query; +use ipl\Sql\Connection; class DbMigration extends MigrationHook { - use Database; + use Database { + getDb as public getPublicDb; + } + + public function getDb(): Connection + { + return $this->getPublicDb(); + } public function getName(): string { diff --git a/public/css/icinga/pending-migration.less b/public/css/icinga/pending-migration.less index 96f693bd4..512183167 100644 --- a/public/css/icinga/pending-migration.less +++ b/public/css/icinga/pending-migration.less @@ -2,13 +2,11 @@ @visual-width: 1.5em; -.migration-state-banner { +.migration-state-banner, .change-database-user-description { .rounded-corners(); - border: 1px solid @gray-lighter; - color: @text-color-light; - padding: 1em; - text-align: center; + border: 1px solid @gray-light; + color: @text-color; } .migrations { @@ -45,6 +43,17 @@ } // Layout + +.migration-state-banner, .change-database-user-description { + padding: 1em; + text-align: center; + + &.change-database-user-description { + max-width: 50em; + padding: .5em; + } +} + .pending-migrations-hint { text-align: center; @@ -63,6 +72,10 @@ .icinga-form { // Reset Icinga Form layout styles width: unset; max-width: unset; + + fieldset { + max-width: 50em; + } } .migration-list-control { From ce89d4a7cbb8e7bf6fdf9bb94bcbeaaa0ad52045 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Thu, 14 Sep 2023 17:30:11 +0200 Subject: [PATCH 32/48] Rename `Common\DbMigration` -> `DbMigrationStep` --- .../Common/{DbMigration.php => DbMigrationStep.php} | 4 ++-- library/Icinga/Application/Hook/MigrationHook.php | 10 +++++----- .../Web/Widget/ItemList/MigrationFileListItem.php | 4 ++-- library/Icinga/Web/Widget/ItemList/MigrationList.php | 4 ++-- .../Icinga/Web/Widget/ItemList/MigrationListItem.php | 4 ++-- 5 files changed, 13 insertions(+), 13 deletions(-) rename library/Icinga/Application/Hook/Common/{DbMigration.php => DbMigrationStep.php} (98%) diff --git a/library/Icinga/Application/Hook/Common/DbMigration.php b/library/Icinga/Application/Hook/Common/DbMigrationStep.php similarity index 98% rename from library/Icinga/Application/Hook/Common/DbMigration.php rename to library/Icinga/Application/Hook/Common/DbMigrationStep.php index ae79da785..97d686f54 100644 --- a/library/Icinga/Application/Hook/Common/DbMigration.php +++ b/library/Icinga/Application/Hook/Common/DbMigrationStep.php @@ -7,7 +7,7 @@ namespace Icinga\Application\Hook\Common; use ipl\Sql\Connection; use RuntimeException; -class DbMigration +class DbMigrationStep { /** @var string The sql string to be executed */ protected $query; @@ -65,7 +65,7 @@ class DbMigration * * @param ?string $description * - * @return DbMigration + * @return DbMigrationStep */ public function setDescription(?string $description): self { diff --git a/library/Icinga/Application/Hook/MigrationHook.php b/library/Icinga/Application/Hook/MigrationHook.php index 4b8c632d0..dc31dd704 100644 --- a/library/Icinga/Application/Hook/MigrationHook.php +++ b/library/Icinga/Application/Hook/MigrationHook.php @@ -9,7 +9,7 @@ use DateTime; use DirectoryIterator; use Exception; use Icinga\Application\ClassLoader; -use Icinga\Application\Hook\Common\DbMigration; +use Icinga\Application\Hook\Common\DbMigrationStep; use Icinga\Application\Icinga; use Icinga\Application\Logger; use Icinga\Application\Modules\Module; @@ -51,7 +51,7 @@ abstract class MigrationHook implements Countable public const ALL_MIGRATIONS = 'all-migrations'; - /** @var ?array All pending database migrations of this hook */ + /** @var ?array All pending database migrations of this hook */ protected $migrations; /** @var ?string The current version of this hook */ @@ -141,7 +141,7 @@ abstract class MigrationHook implements Countable /** * Get all the pending migrations of this hook * - * @return DbMigration[] + * @return DbMigrationStep[] */ public function getMigrations(): array { @@ -159,7 +159,7 @@ abstract class MigrationHook implements Countable * * @param int $limit * - * @return DbMigration[] + * @return DbMigrationStep[] */ public function getLatestMigrations(int $limit): array { @@ -282,7 +282,7 @@ abstract class MigrationHook implements Countable foreach (new DirectoryIterator($path . DIRECTORY_SEPARATOR . $upgradeDir) as $file) { if (preg_match('/^(?:r|v)?((?:\d+\.){0,2}\d+)(?:_([\w+]+))?\.sql$/', $file->getFilename(), $m)) { if (version_compare($m[1], $version, '>')) { - $migration = new DbMigration($m[1], $file->getRealPath()); + $migration = new DbMigrationStep($m[1], $file->getRealPath()); if (isset($descriptions[$migration->getVersion()])) { $migration->setDescription($descriptions[$migration->getVersion()]); } elseif (isset($m[2])) { diff --git a/library/Icinga/Web/Widget/ItemList/MigrationFileListItem.php b/library/Icinga/Web/Widget/ItemList/MigrationFileListItem.php index 9ce63a881..007a730e0 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationFileListItem.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationFileListItem.php @@ -4,7 +4,7 @@ namespace Icinga\Web\Widget\ItemList; -use Icinga\Application\Hook\Common\DbMigration; +use Icinga\Application\Hook\Common\DbMigrationStep; use ipl\Html\Attributes; use ipl\Html\BaseHtmlElement; use ipl\Html\Html; @@ -20,7 +20,7 @@ class MigrationFileListItem extends BaseListItem { use Translation; - /** @var DbMigration Just for type hint */ + /** @var DbMigrationStep Just for type hint */ protected $item; protected function assembleVisual(BaseHtmlElement $visual): void diff --git a/library/Icinga/Web/Widget/ItemList/MigrationList.php b/library/Icinga/Web/Widget/ItemList/MigrationList.php index b8a153b31..cb6147bae 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationList.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationList.php @@ -5,7 +5,7 @@ namespace Icinga\Web\Widget\ItemList; use Generator; -use Icinga\Application\Hook\Common\DbMigration; +use Icinga\Application\Hook\Common\DbMigrationStep; use Icinga\Application\Hook\MigrationHook; use Icinga\Application\MigrationManager; use Icinga\Forms\MigrationForm; @@ -31,7 +31,7 @@ class MigrationList extends BaseItemList /** * Create a new migration list * - * @param Generator|array $data + * @param Generator|array $data * * @param ?MigrationForm $form */ diff --git a/library/Icinga/Web/Widget/ItemList/MigrationListItem.php b/library/Icinga/Web/Widget/ItemList/MigrationListItem.php index d9010db22..641ba5541 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationListItem.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationListItem.php @@ -4,7 +4,7 @@ namespace Icinga\Web\Widget\ItemList; -use Icinga\Application\Hook\Common\DbMigration; +use Icinga\Application\Hook\Common\DbMigrationStep; use Icinga\Application\Hook\MigrationHook; use ipl\Html\Attributes; use ipl\Html\BaseHtmlElement; @@ -69,7 +69,7 @@ class MigrationListItem extends BaseListItem protected function assembleCaption(BaseHtmlElement $caption): void { $migrations = $this->item->getMigrations(); - /** @var DbMigration $migration */ + /** @var DbMigrationStep $migration */ $migration = array_shift($migrations); if ($migration->getLastState()) { if ($migration->getDescription()) { From 26cae8b882b7c6af6285e42d409d5c835b4efd94 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Thu, 14 Sep 2023 17:36:27 +0200 Subject: [PATCH 33/48] Rename `MigrationHook` -> `DbMigrationHook` --- application/controllers/ErrorController.php | 4 +-- .../controllers/MigrationsController.php | 14 +++++----- .../Application/ApplicationBootstrap.php | 2 +- ...{MigrationHook.php => DbMigrationHook.php} | 2 +- .../Icinga/Application/MigrationManager.php | 28 +++++++++---------- .../Application/ProvidedHook/DbMigration.php | 4 +-- .../Web/Widget/ItemList/MigrationList.php | 10 +++---- .../Web/Widget/ItemList/MigrationListItem.php | 6 ++-- 8 files changed, 35 insertions(+), 35 deletions(-) rename library/Icinga/Application/Hook/{MigrationHook.php => DbMigrationHook.php} (99%) diff --git a/application/controllers/ErrorController.php b/application/controllers/ErrorController.php index a22b040b7..476b71f2e 100644 --- a/application/controllers/ErrorController.php +++ b/application/controllers/ErrorController.php @@ -3,7 +3,7 @@ namespace Icinga\Controllers; -use Icinga\Application\Hook\MigrationHook; +use Icinga\Application\Hook\DbMigrationHook; use Icinga\Application\MigrationManager; use Icinga\Exception\IcingaException; use Zend_Controller_Plugin_ErrorHandler; @@ -103,7 +103,7 @@ class ErrorController extends ActionController // safely unset this. $this->setParam('error_handler', null); $this->forward('hint', 'migrations', 'default', [ - MigrationHook::MIGRATION_PARAM => $moduleName + DbMigrationHook::MIGRATION_PARAM => $moduleName ]); return; diff --git a/application/controllers/MigrationsController.php b/application/controllers/MigrationsController.php index e1bcd5c4d..900d50ca4 100644 --- a/application/controllers/MigrationsController.php +++ b/application/controllers/MigrationsController.php @@ -5,7 +5,7 @@ namespace Icinga\Controllers; use Exception; -use Icinga\Application\Hook\MigrationHook; +use Icinga\Application\Hook\DbMigrationHook; use Icinga\Application\Icinga; use Icinga\Application\MigrationManager; use Icinga\Common\Database; @@ -57,10 +57,10 @@ class MigrationsController extends CompatController $migrateListForm->setRenderDatabaseUserChange(! $mm->validateDatabasePrivileges()); $migrateGlobalForm = new MigrationForm(); - $migrateGlobalForm->getAttributes()->set('name', sprintf('migrate-%s', MigrationHook::ALL_MIGRATIONS)); + $migrateGlobalForm->getAttributes()->set('name', sprintf('migrate-%s', DbMigrationHook::ALL_MIGRATIONS)); if ($canApply && $mm->hasPendingMigrations()) { - $migrateGlobalForm->addElement('submit', sprintf('migrate-%s', MigrationHook::ALL_MIGRATIONS), [ + $migrateGlobalForm->addElement('submit', sprintf('migrate-%s', DbMigrationHook::ALL_MIGRATIONS), [ 'required' => true, 'label' => $this->translate('Migrate All'), 'title' => $this->translate('Migrate all pending migrations') @@ -103,9 +103,9 @@ class MigrationsController extends CompatController // The forwarded request doesn't modify the original server query string, but adds the migration param to the // request param instead. So, there is no way to access the migration param other than via the request instance. /** @var ?string $module */ - $module = $this->getRequest()->getParam(MigrationHook::MIGRATION_PARAM); + $module = $this->getRequest()->getParam(DbMigrationHook::MIGRATION_PARAM); if ($module === null) { - throw new MissingParameterException(t('Required parameter \'%s\' missing'), MigrationHook::MIGRATION_PARAM); + throw new MissingParameterException(t('Required parameter \'%s\' missing'), DbMigrationHook::MIGRATION_PARAM); } $mm = MigrationManager::instance(); @@ -134,7 +134,7 @@ class MigrationsController extends CompatController public function migrationAction(): void { /** @var string $name */ - $name = $this->params->getRequired(MigrationHook::MIGRATION_PARAM); + $name = $this->params->getRequired(DbMigrationHook::MIGRATION_PARAM); $this->addTitleTab($this->translate('Migration')); $this->getTabs()->disableLegacyExtensions(); @@ -215,7 +215,7 @@ class MigrationsController extends CompatController if ($pressedButton) { $name = substr($pressedButton->getName(), 8); switch ($name) { - case MigrationHook::ALL_MIGRATIONS: + case DbMigrationHook::ALL_MIGRATIONS: if ($mm->applyAll($elevatedPrivileges)) { Notification::success($this->translate('Applied all migrations successfully')); } else { diff --git a/library/Icinga/Application/ApplicationBootstrap.php b/library/Icinga/Application/ApplicationBootstrap.php index ecc283e65..e484f6c03 100644 --- a/library/Icinga/Application/ApplicationBootstrap.php +++ b/library/Icinga/Application/ApplicationBootstrap.php @@ -740,7 +740,7 @@ abstract class ApplicationBootstrap */ protected function registerApplicationHooks(): self { - Hook::register('migration', DbMigration::class, DbMigration::class); + Hook::register('DbMigration', DbMigration::class, DbMigration::class); return $this; } diff --git a/library/Icinga/Application/Hook/MigrationHook.php b/library/Icinga/Application/Hook/DbMigrationHook.php similarity index 99% rename from library/Icinga/Application/Hook/MigrationHook.php rename to library/Icinga/Application/Hook/DbMigrationHook.php index dc31dd704..dfa048d8b 100644 --- a/library/Icinga/Application/Hook/MigrationHook.php +++ b/library/Icinga/Application/Hook/DbMigrationHook.php @@ -35,7 +35,7 @@ use stdClass; * * `{IcingaApp,Module}::baseDir()/schema/{mysql,pgsql}-upgrades` */ -abstract class MigrationHook implements Countable +abstract class DbMigrationHook implements Countable { use Translation; diff --git a/library/Icinga/Application/MigrationManager.php b/library/Icinga/Application/MigrationManager.php index 6151a84c2..31ea6b20b 100644 --- a/library/Icinga/Application/MigrationManager.php +++ b/library/Icinga/Application/MigrationManager.php @@ -6,7 +6,7 @@ namespace Icinga\Application; use Countable; use Generator; -use Icinga\Application\Hook\MigrationHook; +use Icinga\Application\Hook\DbMigrationHook; use Icinga\Exception\NotFoundError; use Icinga\Module\Setup\Utils\DbTool; use Icinga\Module\Setup\WebWizard; @@ -21,7 +21,7 @@ final class MigrationManager implements Countable { use Translation; - /** @var array All pending migration hooks */ + /** @var array All pending migration hooks */ protected $pendingMigrations; /** @var MigrationManager */ @@ -48,7 +48,7 @@ final class MigrationManager implements Countable /** * Get all pending migrations * - * @return array + * @return array */ public function getPendingMigrations(): array { @@ -83,11 +83,11 @@ final class MigrationManager implements Countable * * @param string $module * - * @return MigrationHook + * @return DbMigrationHook * * @throws NotFoundError When there are no pending migrations matching the given module name */ - public function getMigration(string $module): MigrationHook + public function getMigration(string $module): DbMigrationHook { if (! $this->hasMigrations($module)) { throw new NotFoundError('There are no pending migrations matching the given name: %s', $module); @@ -116,7 +116,7 @@ final class MigrationManager implements Countable public function applyByName(string $module): bool { $migration = $this->getMigration($module); - if ($migration->isModule() && $this->hasMigrations(MigrationHook::DEFAULT_MODULE)) { + if ($migration->isModule() && $this->hasMigrations(DbMigrationHook::DEFAULT_MODULE)) { return false; } @@ -126,14 +126,14 @@ final class MigrationManager implements Countable /** * Apply the given migration hook * - * @param MigrationHook $hook + * @param DbMigrationHook $hook * @param ?array $elevateConfig * * @return bool */ - public function apply(MigrationHook $hook, array $elevateConfig = null): bool + public function apply(DbMigrationHook $hook, array $elevateConfig = null): bool { - if ($hook->isModule() && $this->hasMigrations(MigrationHook::DEFAULT_MODULE)) { + if ($hook->isModule() && $this->hasMigrations(DbMigrationHook::DEFAULT_MODULE)) { Logger::error( 'Please apply the Icinga Web pending migration(s) first or apply all the migrations instead' ); @@ -166,7 +166,7 @@ final class MigrationManager implements Countable */ public function applyAll(array $elevateConfig = null): bool { - $default = MigrationHook::DEFAULT_MODULE; + $default = DbMigrationHook::DEFAULT_MODULE; if ($this->hasMigrations($default)) { $migration = $this->getMigration($default); if (! $this->apply($migration, $elevateConfig)) { @@ -189,7 +189,7 @@ final class MigrationManager implements Countable * * @param bool $modules * - * @return Generator + * @return Generator */ public function yieldMigrations(bool $modules = false): Generator { @@ -297,8 +297,8 @@ final class MigrationManager implements Countable { $this->pendingMigrations = []; - /** @var MigrationHook $hook */ - foreach (Hook::all('migration') as $hook) { + /** @var DbMigrationHook $hook */ + foreach (Hook::all('DbMigration') as $hook) { if (empty($hook->getMigrations())) { continue; } @@ -366,7 +366,7 @@ final class MigrationManager implements Countable public function toArray(): array { $framework = []; - $serialize = function (MigrationHook $hook): array { + $serialize = function (DbMigrationHook $hook): array { $serialized = [ 'name' => $hook->getName(), 'module' => $hook->getModuleName(), diff --git a/library/Icinga/Application/ProvidedHook/DbMigration.php b/library/Icinga/Application/ProvidedHook/DbMigration.php index 37d299535..06fd06f03 100644 --- a/library/Icinga/Application/ProvidedHook/DbMigration.php +++ b/library/Icinga/Application/ProvidedHook/DbMigration.php @@ -4,13 +4,13 @@ namespace Icinga\Application\ProvidedHook; -use Icinga\Application\Hook\MigrationHook; +use Icinga\Application\Hook\DbMigrationHook; use Icinga\Common\Database; use Icinga\Model\Schema; use ipl\Orm\Query; use ipl\Sql\Connection; -class DbMigration extends MigrationHook +class DbMigration extends DbMigrationHook { use Database { getDb as public getPublicDb; diff --git a/library/Icinga/Web/Widget/ItemList/MigrationList.php b/library/Icinga/Web/Widget/ItemList/MigrationList.php index cb6147bae..43699d3e5 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationList.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationList.php @@ -6,7 +6,7 @@ namespace Icinga\Web\Widget\ItemList; use Generator; use Icinga\Application\Hook\Common\DbMigrationStep; -use Icinga\Application\Hook\MigrationHook; +use Icinga\Application\Hook\DbMigrationHook; use Icinga\Application\MigrationManager; use Icinga\Forms\MigrationForm; use ipl\I18n\Translation; @@ -19,7 +19,7 @@ class MigrationList extends BaseItemList protected $baseAttributes = ['class' => 'item-list']; - /** @var Generator */ + /** @var Generator */ protected $data; /** @var ?MigrationForm */ @@ -31,7 +31,7 @@ class MigrationList extends BaseItemList /** * Create a new migration list * - * @param Generator|array $data + * @param Generator|array $data * * @param ?MigrationForm $form */ @@ -82,7 +82,7 @@ class MigrationList extends BaseItemList $this->getAttributes()->add('class', 'file-list'); } - /** @var MigrationHook $data */ + /** @var DbMigrationHook $data */ foreach ($this->data as $data) { /** @var MigrationFileListItem|MigrationListItem $item */ $item = new $itemClass($data, $this); @@ -105,7 +105,7 @@ class MigrationList extends BaseItemList ); $mm = MigrationManager::instance(); - if ($data->isModule() && $mm->hasMigrations(MigrationHook::DEFAULT_MODULE)) { + if ($data->isModule() && $mm->hasMigrations(DbMigrationHook::DEFAULT_MODULE)) { $migrateButton->getAttributes() ->set('disabled', true) ->set( diff --git a/library/Icinga/Web/Widget/ItemList/MigrationListItem.php b/library/Icinga/Web/Widget/ItemList/MigrationListItem.php index 641ba5541..284ce4c84 100644 --- a/library/Icinga/Web/Widget/ItemList/MigrationListItem.php +++ b/library/Icinga/Web/Widget/ItemList/MigrationListItem.php @@ -5,7 +5,7 @@ namespace Icinga\Web\Widget\ItemList; use Icinga\Application\Hook\Common\DbMigrationStep; -use Icinga\Application\Hook\MigrationHook; +use Icinga\Application\Hook\DbMigrationHook; use ipl\Html\Attributes; use ipl\Html\BaseHtmlElement; use ipl\Html\Contract\FormElement; @@ -29,7 +29,7 @@ class MigrationListItem extends BaseListItem /** @var ?FormElement */ protected $migrateButton; - /** @var MigrationHook Just for type hint */ + /** @var DbMigrationHook Just for type hint */ protected $item; /** @@ -124,7 +124,7 @@ class MigrationListItem extends BaseListItem sprintf($this->translate('Show all %d migrations'), $this->item->count()), Url::fromPath( 'migrations/migration', - [MigrationHook::MIGRATION_PARAM => $this->item->getModuleName()] + [DbMigrationHook::MIGRATION_PARAM => $this->item->getModuleName()] ), [ 'data-base-target' => '_next', From 864a78302f9533afd93020704161e04c3fed4ac5 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Thu, 14 Sep 2023 17:37:30 +0200 Subject: [PATCH 34/48] Make sql schema files consistent --- schema/mysql-upgrades/2.12.0.sql | 2 +- schema/mysql.schema.sql | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/schema/mysql-upgrades/2.12.0.sql b/schema/mysql-upgrades/2.12.0.sql index d72a016c9..cb377df82 100644 --- a/schema/mysql-upgrades/2.12.0.sql +++ b/schema/mysql-upgrades/2.12.0.sql @@ -1,7 +1,7 @@ ALTER TABLE icingaweb_schema MODIFY COLUMN timestamp bigint unsigned NOT NULL, MODIFY COLUMN version varchar(64) NOT NULL, - ADD COLUMN IF NOT EXISTS success enum ('n', 'y') DEFAULT NULL, + ADD COLUMN IF NOT EXISTS success enum('n', 'y') DEFAULT NULL, ADD COLUMN IF NOT EXISTS reason text DEFAULT NULL, DROP CONSTRAINT IF EXISTS idx_icingaweb_schema_version, ADD CONSTRAINT idx_icingaweb_schema_version UNIQUE (version); diff --git a/schema/mysql.schema.sql b/schema/mysql.schema.sql index bb37f9b28..1eb71a8a9 100644 --- a/schema/mysql.schema.sql +++ b/schema/mysql.schema.sql @@ -56,8 +56,8 @@ CREATE TABLE `icingaweb_rememberme`( CREATE TABLE icingaweb_schema ( id int unsigned NOT NULL AUTO_INCREMENT, version varchar(64) NOT NULL, - timestamp bigint NOT NULL, - success enum ('n', 'y') DEFAULT NULL, + timestamp bigint unsigned NOT NULL, + success enum('n', 'y') DEFAULT NULL, reason text DEFAULT NULL, PRIMARY KEY (id), From fac3855a86d57d53f0380d415120b8fc142e50e3 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Fri, 15 Sep 2023 11:17:05 +0200 Subject: [PATCH 35/48] DbMigrationStep: Don't cache sql statements unnecessarily --- .../Hook/Common/DbMigrationStep.php | 37 ++++++++----------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/library/Icinga/Application/Hook/Common/DbMigrationStep.php b/library/Icinga/Application/Hook/Common/DbMigrationStep.php index 97d686f54..54a113972 100644 --- a/library/Icinga/Application/Hook/Common/DbMigrationStep.php +++ b/library/Icinga/Application/Hook/Common/DbMigrationStep.php @@ -9,9 +9,6 @@ use RuntimeException; class DbMigrationStep { - /** @var string The sql string to be executed */ - protected $query; - /** @var string The sql script version the queries are loaded from */ protected $version; @@ -109,27 +106,23 @@ class DbMigrationStep */ public function apply(Connection $conn): self { - if (! $this->query) { - $statements = @file_get_contents($this->getScriptPath()); - if ($statements === false) { - throw new RuntimeException(sprintf('Cannot load upgrade script %s', $this->getScriptPath())); - } - - if (empty($statements)) { - throw new RuntimeException('Nothing to migrate'); - } - - if (preg_match('/\s*delimiter\s*(\S+)\s*$/im', $statements, $matches)) { - /** @var string $statements */ - $statements = preg_replace('/\s*delimiter\s*(\S+)\s*$/im', '', $statements); - /** @var string $statements */ - $statements = preg_replace('/' . preg_quote($matches[1], '/') . '$/m', ';', $statements); - } - - $this->query = $statements; + $statements = @file_get_contents($this->getScriptPath()); + if ($statements === false) { + throw new RuntimeException(sprintf('Cannot load upgrade script %s', $this->getScriptPath())); } - $conn->exec($this->query); + if (empty($statements)) { + throw new RuntimeException('Nothing to migrate'); + } + + if (preg_match('/\s*delimiter\s*(\S+)\s*$/im', $statements, $matches)) { + /** @var string $statements */ + $statements = preg_replace('/\s*delimiter\s*(\S+)\s*$/im', '', $statements); + /** @var string $statements */ + $statements = preg_replace('/' . preg_quote($matches[1], '/') . '$/m', ';', $statements); + } + + $conn->exec($statements); return $this; } From 96a632156961a00a2c01503f5a46dfe1e37b62bd Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Fri, 15 Sep 2023 11:25:33 +0200 Subject: [PATCH 36/48] DbMigration: Adjust usage of `Database::getDb()` --- library/Icinga/Application/ProvidedHook/DbMigration.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/Icinga/Application/ProvidedHook/DbMigration.php b/library/Icinga/Application/ProvidedHook/DbMigration.php index 06fd06f03..0442b1d8f 100644 --- a/library/Icinga/Application/ProvidedHook/DbMigration.php +++ b/library/Icinga/Application/ProvidedHook/DbMigration.php @@ -13,12 +13,12 @@ use ipl\Sql\Connection; class DbMigration extends DbMigrationHook { use Database { - getDb as public getPublicDb; + getDb as private getWebDb; } public function getDb(): Connection { - return $this->getPublicDb(); + return $this->getWebDb(); } public function getName(): string From dc738ec4ce753c8790e59e0d32c30b2d4e444049 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Fri, 15 Sep 2023 11:27:43 +0200 Subject: [PATCH 37/48] `DbMigrationHook`: Adjust regex pattern & add missing argument docs --- .../Application/Hook/DbMigrationHook.php | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/library/Icinga/Application/Hook/DbMigrationHook.php b/library/Icinga/Application/Hook/DbMigrationHook.php index dfa048d8b..fe01474a7 100644 --- a/library/Icinga/Application/Hook/DbMigrationHook.php +++ b/library/Icinga/Application/Hook/DbMigrationHook.php @@ -14,16 +14,13 @@ use Icinga\Application\Icinga; use Icinga\Application\Logger; use Icinga\Application\Modules\Module; use Icinga\Model\Schema; -use Icinga\Module\Setup\Utils\DbTool; use Icinga\Web\Session; use ipl\I18n\Translation; use ipl\Orm\Query; use ipl\Sql\Adapter\Pgsql; use ipl\Sql\Connection; use ipl\Stdlib\Filter; -use ipl\Stdlib\Str; use PDO; -use PDOException; use SplFileInfo; use stdClass; @@ -174,6 +171,9 @@ abstract class DbMigrationHook implements Countable /** * Apply all pending migrations of this hook * + * @param ?Connection $conn Use the provided database connection to apply the migrations. + * Is only used to elevate database users with insufficient privileges. + * * @return bool Whether the migration(s) have been successfully applied */ final public function run(Connection $conn = null): bool @@ -205,12 +205,8 @@ abstract class DbMigrationHook implements Countable ); Logger::debug($e->getTraceAsString()); - $schemaQuery = $this->getSchemaQuery() - ->filter(Filter::equal('version', $migration->getVersion())); - static::insertFailedEntry( $conn, - $schemaQuery, $migration->getVersion(), $e->getMessage() . PHP_EOL . $e->getTraceAsString() ); @@ -280,18 +276,19 @@ abstract class DbMigrationHook implements Countable $version = $this->getVersion(); /** @var SplFileInfo $file */ foreach (new DirectoryIterator($path . DIRECTORY_SEPARATOR . $upgradeDir) as $file) { - if (preg_match('/^(?:r|v)?((?:\d+\.){0,2}\d+)(?:_([\w+]+))?\.sql$/', $file->getFilename(), $m)) { - if (version_compare($m[1], $version, '>')) { - $migration = new DbMigrationStep($m[1], $file->getRealPath()); - if (isset($descriptions[$migration->getVersion()])) { - $migration->setDescription($descriptions[$migration->getVersion()]); - } elseif (isset($m[2])) { - $migration->setDescription(str_replace('_', ' ', $m[2])); + if (preg_match('/^(v)?([^_]+)(?:_(\w+))?\.sql$/', $file->getFilename(), $m, PREG_UNMATCHED_AS_NULL)) { + [$_, $_, $migrateVersion, $description] = $m; + if ($migrateVersion && version_compare($migrateVersion, $version, '>')) { + $migration = new DbMigrationStep($migrateVersion, $file->getRealPath()); + if (isset($descriptions[$migrateVersion])) { + $migration->setDescription($descriptions[$migrateVersion]); + } elseif ($description) { + $migration->setDescription(str_replace('_', ' ', $description)); } - $migration->setLastState($this->loadLastState($migration->getVersion())); + $migration->setLastState($this->loadLastState($migrateVersion)); - $this->migrations[$m[1]] = $migration; + $this->migrations[$migrateVersion] = $migration; } } } @@ -308,14 +305,16 @@ abstract class DbMigrationHook implements Countable * Insert failed migration entry into the database or to the session * * @param Connection $conn - * @param Query $schemaQuery * @param string $version * @param string $reason * * @return $this */ - protected function insertFailedEntry(Connection $conn, Query $schemaQuery, string $version, string $reason): self + protected function insertFailedEntry(Connection $conn, string $version, string $reason): self { + $schemaQuery = $this->getSchemaQuery() + ->filter(Filter::equal('version', $version)); + if (! static::getColumnType($conn, $schemaQuery->getModel()->getTableName(), 'success')) { $this->storeState($version, $reason); } else { From 6a4314120b58468934950ffa4562fc51a18f9c26 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Fri, 15 Sep 2023 11:28:54 +0200 Subject: [PATCH 38/48] Don't use `IF (NOT) EXITS` SQL commands in upgrade scripts --- schema/mysql-upgrades/2.12.0.sql | 8 +++----- schema/pgsql-upgrades/2.12.0.sql | 4 +--- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/schema/mysql-upgrades/2.12.0.sql b/schema/mysql-upgrades/2.12.0.sql index cb377df82..f2630ac34 100644 --- a/schema/mysql-upgrades/2.12.0.sql +++ b/schema/mysql-upgrades/2.12.0.sql @@ -1,13 +1,11 @@ ALTER TABLE icingaweb_schema MODIFY COLUMN timestamp bigint unsigned NOT NULL, MODIFY COLUMN version varchar(64) NOT NULL, - ADD COLUMN IF NOT EXISTS success enum('n', 'y') DEFAULT NULL, - ADD COLUMN IF NOT EXISTS reason text DEFAULT NULL, - DROP CONSTRAINT IF EXISTS idx_icingaweb_schema_version, + ADD COLUMN success enum('n', 'y') DEFAULT NULL, + ADD COLUMN reason text DEFAULT NULL, ADD CONSTRAINT idx_icingaweb_schema_version UNIQUE (version); UPDATE icingaweb_schema SET timestamp = timestamp * 1000, success = 'y'; INSERT INTO icingaweb_schema (version, timestamp, success, reason) - VALUES('2.12.0', UNIX_TIMESTAMP() * 1000, 'y', NULL) - ON DUPLICATE KEY UPDATE timestamp = VALUES(timestamp), success = VALUES(success), reason = VALUES(reason); + VALUES('2.12.0', UNIX_TIMESTAMP() * 1000, 'y', NULL); diff --git a/schema/pgsql-upgrades/2.12.0.sql b/schema/pgsql-upgrades/2.12.0.sql index 706a28c69..2a5818e5e 100644 --- a/schema/pgsql-upgrades/2.12.0.sql +++ b/schema/pgsql-upgrades/2.12.0.sql @@ -5,11 +5,9 @@ ALTER TABLE icingaweb_schema ALTER COLUMN version TYPE varchar(64), ADD COLUMN success boolenum DEFAULT NULL, ADD COLUMN reason text DEFAULT NULL, - DROP CONSTRAINT IF EXISTS idx_icingaweb_schema_version, ADD CONSTRAINT idx_icingaweb_schema_version UNIQUE (version); UPDATE icingaweb_schema SET timestamp = timestamp * 1000, success = 'y'; INSERT INTO icingaweb_schema (version, timestamp, success, reason) - VALUES('2.12.0', EXTRACT(EPOCH FROM now()) * 1000, 'y', NULL) - ON CONFLICT ON CONSTRAINT idx_icingaweb_schema_version DO UPDATE SET timestamp = EXCLUDED.timestamp, success = EXCLUDED.success, reason = EXCLUDED.reason; + VALUES('2.12.0', EXTRACT(EPOCH FROM now()) * 1000, 'y', NULL); From 3f372330b3b46b66359d54f3e4fa925d37813dd3 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Fri, 15 Sep 2023 11:49:59 +0200 Subject: [PATCH 39/48] CSS: Remove obsolete `icinga-form` styles & store max view width in a variable --- public/css/icinga/pending-migration.less | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/public/css/icinga/pending-migration.less b/public/css/icinga/pending-migration.less index 512183167..f9e112848 100644 --- a/public/css/icinga/pending-migration.less +++ b/public/css/icinga/pending-migration.less @@ -1,6 +1,7 @@ // Style @visual-width: 1.5em; +@max-view-width: 50em; .migration-state-banner, .change-database-user-description { .rounded-corners(); @@ -69,20 +70,15 @@ } .migrations { - .icinga-form { // Reset Icinga Form layout styles - width: unset; - max-width: unset; - - fieldset { - max-width: 50em; - } + .migration-form fieldset { + max-width: @max-view-width; } .migration-list-control { padding-bottom: 1em; > .item-list { - max-width: 50em; + max-width: @max-view-width; } } From 2505e79a2dd41535bde31d524931dd63d44760a7 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Fri, 15 Sep 2023 12:12:17 +0200 Subject: [PATCH 40/48] DbMigration: Check for mysql collation name whether to check 2.11 is migrated --- .../Application/Hook/DbMigrationHook.php | 29 +++++++++++++++++++ .../Application/ProvidedHook/DbMigration.php | 4 ++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/library/Icinga/Application/Hook/DbMigrationHook.php b/library/Icinga/Application/Hook/DbMigrationHook.php index fe01474a7..eb05cd8ad 100644 --- a/library/Icinga/Application/Hook/DbMigrationHook.php +++ b/library/Icinga/Application/Hook/DbMigrationHook.php @@ -107,6 +107,35 @@ abstract class DbMigrationHook implements Countable return $result->column_type; } + /** + * Get the mysql collation name of the given column of the specified table + * + * @param Connection $conn + * @param string $table + * @param string $column + * + * @return ?string + */ + public static function getColumnCollation(Connection $conn, string $table, string $column): ?string + { + if ($conn->getAdapter() instanceof Pgsql) { + return null; + } + + $pdoStmt = $conn->prepexec( + 'SELECT collation_name FROM information_schema.columns WHERE table_name = ? AND column_name = ?', + [$table, $column] + ); + + /** @var false|stdClass $result */ + $result = $pdoStmt->fetch(PDO::FETCH_OBJ); + if ($result === false) { + return null; + } + + return $result->collation_name; + } + /** * Get statically provided descriptions of the individual migrate scripts * diff --git a/library/Icinga/Application/ProvidedHook/DbMigration.php b/library/Icinga/Application/ProvidedHook/DbMigration.php index 0442b1d8f..899dbf6ac 100644 --- a/library/Icinga/Application/ProvidedHook/DbMigration.php +++ b/library/Icinga/Application/ProvidedHook/DbMigration.php @@ -52,7 +52,9 @@ class DbMigration extends DbMigrationHook if (! $this->version) { $this->version = '2.12.0'; } - } elseif (static::tableExists($conn, $schemaQuery->getModel()->getTableName())) { + } elseif (static::tableExists($conn, $schemaQuery->getModel()->getTableName()) + || static::getColumnCollation($conn, 'icingaweb_user_preference', 'username') === 'utf8mb4_unicode_ci' + ) { $this->version = '2.11.0'; } elseif (static::tableExists($conn, 'icingaweb_rememberme')) { $randomIvType = static::getColumnType($conn, 'icingaweb_rememberme', 'random_iv'); From 501ab814165f46c435bbdd8d35cea2677cca27b0 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Fri, 15 Sep 2023 12:19:33 +0200 Subject: [PATCH 41/48] docs: Add missing grants in MYSQL manual setup examples --- doc/20-Advanced-Topics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/20-Advanced-Topics.md b/doc/20-Advanced-Topics.md index 6835a6f28..d88476847 100644 --- a/doc/20-Advanced-Topics.md +++ b/doc/20-Advanced-Topics.md @@ -213,7 +213,7 @@ Create the database and add a new user as shown below for MySQL/MariaDB: sudo mysql -p CREATE DATABASE icingaweb2; -GRANT SELECT, INSERT, UPDATE, DELETE, DROP, CREATE VIEW, INDEX, EXECUTE ON icingaweb2.* TO 'icingaweb2'@'localhost' IDENTIFIED BY 'icingaweb2'; +GRANT CREATE, SELECT, INSERT, UPDATE, DELETE, DROP, ALTER, CREATE VIEW, INDEX, EXECUTE ON icingaweb2.* TO 'icingaweb2'@'localhost' IDENTIFIED BY 'icingaweb2'; quit mysql -p icingaweb2 < /usr/share/icingaweb2/schema/mysql.schema.sql From 99e8a2322d86f4b66e05b5b1ea3bff27d7a6cbf9 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Fri, 15 Sep 2023 12:48:23 +0200 Subject: [PATCH 42/48] Don't render migrate button in detailed file list view --- .../controllers/MigrationsController.php | 32 ++++--------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/application/controllers/MigrationsController.php b/application/controllers/MigrationsController.php index 900d50ca4..1260a870b 100644 --- a/application/controllers/MigrationsController.php +++ b/application/controllers/MigrationsController.php @@ -4,7 +4,6 @@ namespace Icinga\Controllers; -use Exception; use Icinga\Application\Hook\DbMigrationHook; use Icinga\Application\Icinga; use Icinga\Application\MigrationManager; @@ -105,7 +104,10 @@ class MigrationsController extends CompatController /** @var ?string $module */ $module = $this->getRequest()->getParam(DbMigrationHook::MIGRATION_PARAM); if ($module === null) { - throw new MissingParameterException(t('Required parameter \'%s\' missing'), DbMigrationHook::MIGRATION_PARAM); + throw new MissingParameterException( + $this->translate('Required parameter \'%s\' missing'), + DbMigrationHook::MIGRATION_PARAM + ); } $mm = MigrationManager::instance(); @@ -161,33 +163,11 @@ class MigrationsController extends CompatController ) ); } else { - $migrateForm = (new MigrationForm()) - ->addElement( - 'submit', - sprintf('migrate-%s', $hook->getModuleName()), - [ - 'required' => true, - 'label' => $this->translate('Migrate'), - 'title' => sprintf( - $this->translatePlural( - 'Migrate %d pending migration', - 'Migrate all %d pending migrations', - $hook->count() - ), - $hook->count() - ) - ] - ); - - $migrateForm->getAttributes()->add('class', 'inline'); - $this->handleMigrateRequest($migrateForm); - $this->addControl( new HtmlElement( 'div', Attributes::create(['class' => 'migration-controls']), - new HtmlElement('span', null, Text::create($hook->getName())), - $migrateForm + new HtmlElement('span', null, Text::create($hook->getName())) ) ); } @@ -239,6 +219,8 @@ class MigrationsController extends CompatController } } + $this->sendExtraUpdates(['#col2' => '__CLOSE__']); + $this->redirectNow('migrations'); })->handleRequest($this->getServerRequest()); } From d2ce60d4c046dbeb274b1132967b4f1fca6f9f49 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Fri, 15 Sep 2023 12:49:20 +0200 Subject: [PATCH 43/48] Always right align `control-label-group` --- public/css/icinga/pending-migration.less | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/public/css/icinga/pending-migration.less b/public/css/icinga/pending-migration.less index f9e112848..bc70caa33 100644 --- a/public/css/icinga/pending-migration.less +++ b/public/css/icinga/pending-migration.less @@ -45,6 +45,10 @@ // Layout +#layout.twocols:not(.wide-layout) .migration-form fieldset .control-label-group { + text-align: right; +} + .migration-state-banner, .change-database-user-description { padding: 1em; text-align: center; From 47b214ee1bb58419681a0f5b61be27e324afd42d Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Fri, 15 Sep 2023 14:04:59 +0200 Subject: [PATCH 44/48] Use `PDO::fetchColumn()` where applicable --- .../Application/Hook/DbMigrationHook.php | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/library/Icinga/Application/Hook/DbMigrationHook.php b/library/Icinga/Application/Hook/DbMigrationHook.php index eb05cd8ad..5b35f3ccf 100644 --- a/library/Icinga/Application/Hook/DbMigrationHook.php +++ b/library/Icinga/Application/Hook/DbMigrationHook.php @@ -64,13 +64,13 @@ abstract class DbMigrationHook implements Countable */ public static function tableExists(Connection $conn, string $table): bool { - /** @var stdClass $query */ - $query = $conn->prepexec( + /** @var false|int $exists */ + $exists = $conn->prepexec( 'SELECT EXISTS(SELECT 1 FROM information_schema.tables WHERE table_name = ?) AS result', $table - )->fetch(PDO::FETCH_OBJ); + )->fetchColumn(); - return $query->result; + return (bool) $exists; } /** @@ -122,18 +122,13 @@ abstract class DbMigrationHook implements Countable return null; } - $pdoStmt = $conn->prepexec( + /** @var false|string $collation */ + $collation = $conn->prepexec( 'SELECT collation_name FROM information_schema.columns WHERE table_name = ? AND column_name = ?', [$table, $column] - ); + )->fetchColumn(); - /** @var false|stdClass $result */ - $result = $pdoStmt->fetch(PDO::FETCH_OBJ); - if ($result === false) { - return null; - } - - return $result->collation_name; + return ! $collation ? null : $collation; } /** From 167ff5494796e0cbe436f519a65a7450b25c5a3c Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Fri, 15 Sep 2023 15:01:41 +0200 Subject: [PATCH 45/48] Enhance logging --- .../Icinga/Application/Hook/DbMigrationHook.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/library/Icinga/Application/Hook/DbMigrationHook.php b/library/Icinga/Application/Hook/DbMigrationHook.php index 5b35f3ccf..892bb6e97 100644 --- a/library/Icinga/Application/Hook/DbMigrationHook.php +++ b/library/Icinga/Application/Hook/DbMigrationHook.php @@ -213,10 +213,14 @@ abstract class DbMigrationHook implements Countable $this->version = $migration->getVersion(); unset($this->migrations[$migration->getVersion()]); - Logger::info( - "Applied %s pending migration version %s successfully", - $this->getName(), - $migration->getVersion() + $data = [ + 'name' => $this->getName(), + 'version' => $migration->getVersion() + ]; + AuditHook::logActivity( + 'migrations', + 'Migrated database schema of {{name}} to version {{version}}', + $data ); $this->storeState($migration->getVersion(), null); @@ -225,7 +229,7 @@ abstract class DbMigrationHook implements Countable "Failed to apply %s pending migration version %s \n%s", $this->getName(), $migration->getVersion(), - $e->getMessage() + $e ); Logger::debug($e->getTraceAsString()); From 8a1c224461e1a2b87d8ab9d5ae626a7b4c936069 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 15 Sep 2023 16:51:16 +0200 Subject: [PATCH 46/48] WebWizard: Grant permission for DDL statements by default --- modules/setup/library/Setup/WebWizard.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/modules/setup/library/Setup/WebWizard.php b/modules/setup/library/Setup/WebWizard.php index caf6871df..4fc0f2bde 100644 --- a/modules/setup/library/Setup/WebWizard.php +++ b/modules/setup/library/Setup/WebWizard.php @@ -78,6 +78,11 @@ class WebWizard extends Wizard implements SetupWizard 'UPDATE', 'DELETE', 'EXECUTE', + 'CREATE', + 'CREATE VIEW', + 'ALTER', + 'DROP', + 'INDEX', 'TEMPORARY', // PostgreSql 'CREATE TEMPORARY TABLES' // MySQL ); From 4a8d171aec989c1a9e2118d8651ebd4bb3d09b39 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 15 Sep 2023 16:56:22 +0200 Subject: [PATCH 47/48] migrations/index: Let the migrate all button submit the migration form --- .../controllers/MigrationsController.php | 24 +++++++++++-------- application/forms/MigrationForm.php | 10 ++++++-- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/application/controllers/MigrationsController.php b/application/controllers/MigrationsController.php index 1260a870b..5229f066c 100644 --- a/application/controllers/MigrationsController.php +++ b/application/controllers/MigrationsController.php @@ -14,6 +14,7 @@ use Icinga\Web\Notification; use Icinga\Web\Widget\ItemList\MigrationList; use Icinga\Web\Widget\Tabextension\OutputFormat; use ipl\Html\Attributes; +use ipl\Html\FormElement\SubmitButtonElement; use ipl\Html\HtmlElement; use ipl\Html\Text; use ipl\Web\Compat\CompatController; @@ -53,22 +54,25 @@ class MigrationsController extends CompatController } $migrateListForm = new MigrationForm(); + $migrateListForm->setAttribute('id', $this->getRequest()->protectId('migration-form')); $migrateListForm->setRenderDatabaseUserChange(! $mm->validateDatabasePrivileges()); - $migrateGlobalForm = new MigrationForm(); - $migrateGlobalForm->getAttributes()->set('name', sprintf('migrate-%s', DbMigrationHook::ALL_MIGRATIONS)); - if ($canApply && $mm->hasPendingMigrations()) { - $migrateGlobalForm->addElement('submit', sprintf('migrate-%s', DbMigrationHook::ALL_MIGRATIONS), [ - 'required' => true, - 'label' => $this->translate('Migrate All'), - 'title' => $this->translate('Migrate all pending migrations') + $migrateAllButton = new SubmitButtonElement(sprintf('migrate-%s', DbMigrationHook::ALL_MIGRATIONS), [ + 'form' => $migrateListForm->getAttribute('id')->getValue(), + 'label' => $this->translate('Migrate All'), + 'title' => $this->translate('Migrate all pending migrations') ]); - $this->controls->getAttributes()->add('class', 'default-layout'); - $this->handleMigrateRequest($migrateGlobalForm); + // Is the first button, so will be cloned and that the visible + // button is outside the form doesn't matter for Web's JS + $migrateListForm->registerElement($migrateAllButton); - $this->addControl($migrateGlobalForm); + // Make sure it looks familiar, even if not inside a form + $migrateAllButton->setWrapper(new HtmlElement('div', Attributes::create(['class' => 'icinga-controls']))); + + $this->controls->getAttributes()->add('class', 'default-layout'); + $this->addControl($migrateAllButton); } $this->handleFormatRequest($mm->toArray()); diff --git a/application/forms/MigrationForm.php b/application/forms/MigrationForm.php index 120c84684..c5d517f03 100644 --- a/application/forms/MigrationForm.php +++ b/application/forms/MigrationForm.php @@ -6,7 +6,6 @@ namespace Icinga\Forms; use Icinga\Application\MigrationManager; use ipl\Html\Attributes; -use ipl\Html\Form; use ipl\Html\FormElement\CheckboxElement; use ipl\Html\FormElement\FieldsetElement; use ipl\Html\HtmlElement; @@ -15,10 +14,11 @@ use ipl\I18n\Translation; use ipl\Validator\CallbackValidator; use ipl\Web\Common\CsrfCounterMeasure; use ipl\Web\Common\FormUid; +use ipl\Web\Compat\CompatForm; use ipl\Web\FormDecorator\IcingaFormDecorator; use PDOException; -class MigrationForm extends Form +class MigrationForm extends CompatForm { use CsrfCounterMeasure; use FormUid; @@ -50,6 +50,12 @@ class MigrationForm extends Form return $this; } + public function hasDefaultElementDecorator() + { + // The base implementation registers a decorator we don't want here + return false; + } + protected function assemble(): void { $this->addHtml($this->createUidElement()); From 9c6d930e1752a79c77f63e9f9823f6ebf43dd00f Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 15 Sep 2023 17:18:14 +0200 Subject: [PATCH 48/48] MigrationManager: Also check table privileges --- library/Icinga/Application/MigrationManager.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/library/Icinga/Application/MigrationManager.php b/library/Icinga/Application/MigrationManager.php index 31ea6b20b..46e19909c 100644 --- a/library/Icinga/Application/MigrationManager.php +++ b/library/Icinga/Application/MigrationManager.php @@ -327,9 +327,16 @@ final class MigrationManager implements Countable $conn = $this->elevateDatabaseConnection($conn, $elevateConfig); } + $wizardProperties = (new ReflectionClass(WebWizard::class)) + ->getDefaultProperties(); + /** @var array $tables */ + $tables = $wizardProperties['databaseTables']; + $dbTool = $this->createDbTool($conn); $dbTool->connectToDb(); - if (! $dbTool->checkPrivileges($this->getRequiredDatabasePrivileges())) { + if (! $dbTool->checkPrivileges($this->getRequiredDatabasePrivileges()) + && ! $dbTool->checkPrivileges($this->getRequiredDatabasePrivileges(), $tables) + ) { return false; }