From 3bc4f901223c9782dd688a4f4993481e2cdb9b94 Mon Sep 17 00:00:00 2001 From: Jakub Onderka Date: Tue, 3 Aug 2021 14:38:50 +0200 Subject: [PATCH] new: [test] Check schema diagnostics in CI --- .github/workflows/main.yml | 1 + app/Console/Command/AdminShell.php | 34 ++++++++++++++++++++++++++++ app/Controller/ServersController.php | 3 --- app/Model/Server.php | 29 ++++++++++++------------ 4 files changed, 50 insertions(+), 17 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 184ff0d78..f7727ba85 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -135,6 +135,7 @@ jobs: run: | sudo -E su $USER -c 'app/Console/cake Admin setSetting "MISP.osuser" $USER' sudo -E su $USER -c 'app/Console/cake Admin runUpdates' + sudo -E su $USER -c 'app/Console/cake Admin schemaDiagnostics' - name: Configure MISP run: | diff --git a/app/Console/Command/AdminShell.php b/app/Console/Command/AdminShell.php index 03014881b..2eb8c618d 100644 --- a/app/Console/Command/AdminShell.php +++ b/app/Console/Command/AdminShell.php @@ -732,4 +732,38 @@ class AdminShell extends AppShell $message = __('The upgrade process is complete, %s authkey(s) generated.', $updated); $this->out($message); } + + public function schemaDiagnostics() + { + $dbSchemaDiagnostics = $this->Server->dbSchemaDiagnostic(); + $this->out('# Columns diagnostics'); + + foreach ($dbSchemaDiagnostics['diagnostic'] as $tableName => $diagnostics) { + $diagnostics = array_filter($diagnostics, function ($c) { + return $c['is_critical']; + }); + if (empty($diagnostics)) { + continue; + } + $this->out(); + $this->out('Table ' . $tableName . ':'); + foreach ($diagnostics as $diagnostic) { + $this->out(' - ' . $diagnostic['description']); + $this->out(' Expected: ' . implode(' ', $diagnostic['expected'])); + if (!empty($diagnostic['actual'])) { + $this->out(' Actual: ' . implode(' ', $diagnostic['actual'])); + } + } + } + + $this->out(); + $this->out('# Index diagnostics'); + foreach ($dbSchemaDiagnostics['diagnostic_index'] as $tableName => $diagnostics) { + $this->out(); + $this->out('Table ' . $tableName . ':'); + foreach ($diagnostics as $info) { + $this->out(' - ' . $info['message']); + } + } + } } diff --git a/app/Controller/ServersController.php b/app/Controller/ServersController.php index 23372e89d..224c7d679 100644 --- a/app/Controller/ServersController.php +++ b/app/Controller/ServersController.php @@ -2429,9 +2429,6 @@ misp.direct_call(relative_path, body) public function dbSchemaDiagnostic() { - if (!$this->_isSiteAdmin()) { - throw new MethodNotAllowedException(__('Only site admin accounts get the DB schema diagnostic.')); - } $dbSchemaDiagnostics = $this->Server->dbSchemaDiagnostic(); if ($this->_isRest()) { return $this->RestResponse->viewData($dbSchemaDiagnostics, $this->response->type()); diff --git a/app/Model/Server.php b/app/Model/Server.php index 9bc1195c5..97ed8fcfb 100644 --- a/app/Model/Server.php +++ b/app/Model/Server.php @@ -2794,7 +2794,7 @@ class Server extends AppModel } $schemaDiagnostic['indexes'] = $dbActualSchema['indexes']; } else { - $schemaDiagnostic['error'] = sprintf('Diagnostic not available as the expected schema file could not be loaded'); + $schemaDiagnostic['error'] = 'Diagnostic not available as the expected schema file could not be loaded'; } } else { $schemaDiagnostic['error'] = sprintf('Diagnostic not available for DataSource `%s`', $dataSource); @@ -2942,7 +2942,7 @@ class Server extends AppModel $dbActualSchema = array(); $dbActualIndexes = array(); $dataSource = $this->getDataSource()->config['datasource']; - if ($dataSource == 'Database/Mysql' || $dataSource == 'Database/MysqlObserver') { + if ($dataSource === 'Database/Mysql' || $dataSource === 'Database/MysqlObserver') { $sqlGetTable = sprintf('SELECT TABLE_NAME FROM information_schema.tables WHERE table_schema = %s ORDER BY TABLE_NAME;', "'" . $this->getDataSource()->config['database'] . "'"); $sqlResult = $this->query($sqlGetTable); $tables = Hash::extract($sqlResult, '{n}.tables.TABLE_NAME'); @@ -2952,9 +2952,10 @@ class Server extends AppModel FROM information_schema.columns WHERE table_schema = '%s' AND TABLE_NAME = '%s'", implode(',', $tableColumnNames), $this->getDataSource()->config['database'], $table); $sqlResult = $this->query($sqlSchema); + $sqlResult = array_column($sqlResult, 'columns'); foreach ($sqlResult as $column_schema) { - $column_schema['columns'] = array_change_key_case($column_schema['columns'],CASE_LOWER); - $dbActualSchema[$table][] = $column_schema['columns']; + $column_schema = array_change_key_case($column_schema,CASE_LOWER); + $dbActualSchema[$table][] = $column_schema; } $dbActualIndexes[$table] = $this->getDatabaseIndexes($this->getDataSource()->config['database'], $table); } @@ -2962,10 +2963,10 @@ class Server extends AppModel else if ($dataSource == 'Database/Postgres') { return array('Database/Postgres' => array('description' => __('Can\'t check database schema for Postgres database type'))); } - return array('schema' => $dbActualSchema, 'column' => $tableColumnNames, 'indexes' => $dbActualIndexes); + return ['schema' => $dbActualSchema, 'column' => $tableColumnNames, 'indexes' => $dbActualIndexes]; } - public function compareDBSchema($dbActualSchema, $dbExpectedSchema) + private function compareDBSchema($dbActualSchema, $dbExpectedSchema) { // Column that should be ignored while performing the comparison $allowedlistFields = array( @@ -2977,7 +2978,7 @@ class Server extends AppModel foreach($dbExpectedSchema as $tableName => $columns) { if (!array_key_exists($tableName, $dbActualSchema)) { $dbDiff[$tableName][] = array( - 'description' => sprintf(__('Table `%s` does not exist'), $tableName), + 'description' => __('Table `%s` does not exist', $tableName), 'error_type' => 'missing_table', 'expected_table' => $columns, 'column_name' => $tableName, @@ -3004,7 +3005,7 @@ class Server extends AppModel continue; // column is allowedlisted } $dbDiff[$tableName][] = array( - 'description' => sprintf(__('Column `%s` exists but should not'), $additionalKeys), + 'description' => __('Column `%s` exists but should not', $additionalKeys), 'error_type' => 'additional_column', 'column_name' => $additionalKeys, 'is_critical' => false @@ -3037,7 +3038,7 @@ class Server extends AppModel } } $dbDiff[$tableName][] = array( - 'description' => sprintf(__('Column `%s` is different'), $columnName), + 'description' => __('Column `%s` is different', $columnName), 'column_name' => $column['column_name'], 'error_type' => 'column_different', 'actual' => $keyedActualColumn[$columnName], @@ -3047,7 +3048,7 @@ class Server extends AppModel } } else { $dbDiff[$tableName][] = array( - 'description' => sprintf(__('Column `%s` does not exist but should'), $columnName), + 'description' => __('Column `%s` does not exist but should', $columnName), 'column_name' => $columnName, 'error_type' => 'missing_column', 'actual' => array(), @@ -3058,9 +3059,9 @@ class Server extends AppModel } } } - foreach(array_diff(array_keys($dbActualSchema), array_keys($dbExpectedSchema)) as $additionalTable) { + foreach (array_diff(array_keys($dbActualSchema), array_keys($dbExpectedSchema)) as $additionalTable) { $dbDiff[$additionalTable][] = array( - 'description' => sprintf(__('Table `%s` is an additional table'), $additionalTable), + 'description' => __('Table `%s` is an additional table', $additionalTable), 'column_name' => $additionalTable, 'error_type' => 'additional_table', 'is_critical' => false @@ -3118,7 +3119,7 @@ class Server extends AppModel ); } - public function compareDBIndexes(array $actualIndex, array $expectedIndex, array $dbExpectedSchema) + private function compareDBIndexes(array $actualIndex, array $expectedIndex, array $dbExpectedSchema) { $allowedlistTables = array(); $indexDiff = array(); @@ -3198,7 +3199,7 @@ class Server extends AppModel * @param string $table * @return array */ - public function getDatabaseIndexes($database, $table) + private function getDatabaseIndexes($database, $table) { $sqlTableIndex = sprintf( "SELECT DISTINCT TABLE_NAME, COLUMN_NAME, NON_UNIQUE FROM information_schema.statistics WHERE TABLE_SCHEMA = '%s' AND TABLE_NAME = '%s';",