run sql commands in a temp schema by default

Gaetano Giunta · Gaetano Giunta · commit 5955f88c45eb · 2019-04-21T17:28:55.000Z
diff --git a/TODO.md b/TODO.md
@@ -1,8 +1,14 @@
 - worker: improve sql execution cmd:
   + allow it to pick a set of desired servers
+  + allow it to pick an existing db schema & user
   + disallow execution of commands that are part of the db client instead of being sent to the server, such as eg. 'use db'
-    - ok for mysql? (to be tested), missing for psql
-  + examine in detail the differences between running a command vs a file (eg. transaction usage)
+    - ok for mysql? (to be tested)
+    - missing for psql? (according to slack discussion: this is impossible using psql and can only be done using a different
+      driver... it might be in fact already happening when NOT using input via files...)
+  + examine in detail and document the differences between running a command vs a file (eg. transaction usage)
+
+- improve cli scripts:
+  + add scripts to create and drop a schema on all servers
 
 - worker: improve profile of 'user' account (esp: add APP_ENV and APP_DEBUG env vars)
 
@@ -15,9 +21,9 @@
 
 - web: allow to insert sql snippet, pick the desired servers, run it and show results
 
-- web/worker: allow auto db-schema create+teardown (w. user-defined charset)
+- web/worker: allow user-defined charset for auto db-schema create
 
-- web/worker: allow custom db init scripts (load data and set session vars)
+- web/worker: allow custom db init scripts (to load data and set session vars)
 
 - pick up a library which allows to load db-agnostic schema defs and data
 
@@ -35,7 +41,7 @@
   https://www.percona.com/blog/2011/02/01/sample-datasets-for-benchmarking-and-testing/
   https://docs.microsoft.com/en-us/azure/sql-database/sql-database-public-data-sets
 
-- web gui: store previous snippets in a dedicated db, list them
+- web gui: store previous snippets in a dedicated db, list them (private to each user session)
 
 - mariadb/mysql: allow to define in docker parameters the size of the ramdisk used for /tmpfs; 
   also in default configs, do use /tmpfs for temp tables? At least add it commented out
@@ -56,7 +62,7 @@
   - rate-limit http requests
   - size-limit http requests
   - add caching in nginx of static assets
-  - add firewall rules to the web containers to block access to outside world at bootstrap
+  - add firewall rules to the all containers to block access to outside world at bootstrap
   - make php code non-writeable by www-data user
   - harden php configuration
   - move execution of sql snippets to a queue, to avoid dos/overload
diff --git a/WHATSNEW.md b/WHATSNEW.md
@@ -7,6 +7,8 @@ Version 0.3 (unreleased)
     - measure time and memory taken for each db
     - allow to print output in json/yaml/php format
     - allow to execute sql commands stored in a file besides specifying them as cli option
+    - all sql commands are now executed in a temporary db schema, by a corresponding temp. db user, instead of being
+      executed with the database 'root' account 
 
 
 Version 0.2
diff --git a/app/src/Command/SqlExecute.php b/app/src/Command/SqlExecute.php
@@ -8,6 +8,7 @@
 use Symfony\Component\Yaml\Yaml;
 use Db3v4l\API\Interfaces\TimedExecutor;
 use Db3v4l\Service\DatabaseConfigurationManager;
+use Db3v4l\Service\DatabaseSchemaManager;
 use Db3v4l\Service\SqlExecutorFactory;
 use Db3v4l\Service\ProcessManager;
 use Db3v4l\Util\Process;
@@ -37,7 +38,7 @@ public function __construct(
     protected function configure()
     {
         $this
-            ->setDescription('Executes an SQL command in parallel on all configured database servers')
+            ->setDescription('Executes an SQL command in parallel on all configured database servers, creating a dedicated database schema (and user)')
             ->addOption('sql', null, InputOption::VALUE_REQUIRED, 'The sql command(s) string to execute')
             ->addOption('file', null, InputOption::VALUE_REQUIRED, 'A file with sql commands to execute')
             ->addOption('output-type', null, InputOption::VALUE_REQUIRED, 'The format for the output: json, php, text or yml', 'text')
@@ -83,19 +84,23 @@ protected function execute(InputInterface $input, OutputInterface $output)
         // We thus force it, but give end users an option to disable this
         // For more details, see comment 12 at https://bugs.launchpad.net/ubuntu/+source/php5/+bug/516061
         if (!$dontForceSigchildEnabled) {
-
             Process::forceSigchildEnabled(true);
         }
 
+        if ($format === 'text') {
+            $this->writeln('<info>Creating temporary schemas...</info>', OutputInterface::VERBOSITY_VERBOSE);
+        }
+
+        $dbConnectionSpecs = $this->createSchemas($dbList, $maxParallel);
+
         if ($format === 'text') {
             $this->writeln('<info>Preparing commands...</info>', OutputInterface::VERBOSITY_VERBOSE);
         }
 
         /** @var Process[] $processes */
         $processes = [];
         $executors = [];
-        foreach ($dbList as $dbName) {
-            $dbConnectionSpec = $this->dbManager->getDatabaseConnectionSpecification($dbName);
+        foreach ($dbConnectionSpecs as $dbName => $dbConnectionSpec) {
 
             $executor = $this->executorFactory->createForkedExecutor($dbConnectionSpec);
 
@@ -144,11 +149,109 @@ protected function execute(InputInterface $input, OutputInterface $output)
             }
         }
 
+        if ($format === 'text') {
+            $this->writeln('<info>Dropping temporary schemas...</info>', OutputInterface::VERBOSITY_VERBOSE);
+        }
+        $this->dropSchemas($dbConnectionSpecs, $maxParallel);
+
         $time = microtime(true) - $start;
 
         $this->writeResults($results, $succeeded, $failed, $time, $format);
     }
 
+    /**
+     * @param string[] $dbList
+     * @param int $maxParallel
+     * @return array same format as dbManager::getDatabaseConnectionSpecification
+     */
+    protected function createSchemas($dbList, $maxParallel)
+    {
+        $processes = [];
+        $connectionSpecs = [];
+        $tempSQLFileNames = [];
+
+        /// @todo inject more randomness in the username, by allowing more chars than bin2hex produces
+        $userName = bin2hex(random_bytes(8)); // old mysql versions have a limitation of 16 chars for usernames
+        $password = bin2hex(random_bytes(16));
+        //$schemaName = bin2hex(random_bytes(31));
+        $schemaName = null; // $userName will be used as schema name
+
+        foreach ($dbList as $dbName) {
+            $dbConnectionSpec = $this->dbManager->getDatabaseConnectionSpecification($dbName);
+
+            $schemaManager = new DatabaseSchemaManager($dbConnectionSpec);
+            $sql = $schemaManager->getCreateSchemaSQL($userName, $password, $schemaName);
+            // sadly, psql does not allow to create a db and a user using a multiple-sql-commands string,
+            // and we have to resort to using temp files
+            /// @todo can we make this safer? Ideally the new userv name and pwd should neither hit disk nor the process list...
+            $tempSQLFileName = tempnam(sys_get_temp_dir(), 'db3val_');
+            file_put_contents($tempSQLFileName, $sql);
+            $tempSQLFileNames[] = $tempSQLFileName;
+
+            $executor = $this->executorFactory->createForkedExecutor($dbConnectionSpec, 'NativeClient', false);
+            $process = $executor->getExecuteFileProcess($tempSQLFileName);
+            $processes[$dbName] = $process;
+            $connectionSpecs[$dbName] = $dbConnectionSpec;
+        }
+
+        $this->processManager->runParallel($processes, $maxParallel, 100);
+
+        $results = array();
+        foreach ($processes as $dbName => $process) {
+            if ($process->isSuccessful()) {
+                $results[$dbName] = array_merge($connectionSpecs[$dbName], array(
+                    'user' => $userName,
+                    'password' => $password,
+                    'dbname' => $userName
+                ));
+            } else {
+                $this->writeErrorln("\n<error>Creation of new schema & user on database '$dbName' failed! Reason: " . $process->getErrorOutput() . "</error>\n", OutputInterface::VERBOSITY_NORMAL);
+            }
+        }
+
+        foreach($tempSQLFileNames as $tempSQLFileName) {
+            unlink($tempSQLFileName);
+        }
+
+        return $results;
+    }
+
+    /**
+     * @param array $dbSpecList
+     * @param int $maxParallel
+     */
+    protected function dropSchemas($dbSpecList, $maxParallel)
+    {
+        $processes = [];
+        $tempSQLFileNames = [];
+
+        foreach ($dbSpecList as $dbName => $dbConnectionSpec) {
+            $dbConnectionSpec = $this->dbManager->getDatabaseConnectionSpecification($dbName);
+
+            $schemaManager = new DatabaseSchemaManager($dbConnectionSpec);
+            $sql= $schemaManager->getDropSchemaSQL($dbConnectionSpec['user'], isset($dbConnectionSpec['dbname']) ? $dbConnectionSpec['dbname'] : null );
+            $tempSQLFileName = tempnam(sys_get_temp_dir(), 'db3val_');
+            file_put_contents($tempSQLFileName, $sql);
+            $tempSQLFileNames[] = $tempSQLFileName;
+
+            $executor = $this->executorFactory->createForkedExecutor($dbConnectionSpec, 'NativeClient', false);
+            $process = $executor->getExecuteFileProcess($tempSQLFileName);
+            $processes[$dbName] = $process;
+        }
+
+        $this->processManager->runParallel($processes, $maxParallel, 100);
+
+        foreach ($processes as $dbName => $process) {
+            if (!$process->isSuccessful()) {
+                $this->writeErrorln("\n<error>Drop of new schema & user on database '$dbName' failed! Reason: " . $process->getErrorOutput() . "</error>\n", OutputInterface::VERBOSITY_NORMAL);
+            }
+        }
+
+        foreach($tempSQLFileNames as $tempSQLFileName) {
+            unlink($tempSQLFileName);
+        }
+    }
+
     /**
      * @param array $results
      * @param int $succeeded
diff --git a/app/src/Service/DatabaseSchemaManager.php b/app/src/Service/DatabaseSchemaManager.php
@@ -0,0 +1,82 @@
+<?php
+
+namespace Db3v4l\Service;
+
+class DatabaseSchemaManager
+{
+    protected $databaseConfiguration;
+
+    public function __construct(array $databaseConfiguration)
+    {
+        $this->databaseConfiguration = $databaseConfiguration;
+    }
+
+    /**
+     * Returns the sql commands used to create a new db schema and accompanying user
+     * @param string $userName used both for user and schema if passed schemaName is null. Max 16 chars for MySQL 5.5
+     * @param string $password
+     * @param string $schemaName Max 63 chars for Postgres
+     * @param string $charset
+     * @return string
+     */
+    public function getCreateSchemaSQL($userName, $password, $schemaName = null, $charset = null)
+    {
+        if ($schemaName === null) {
+            $schemaName = $userName;
+        }
+
+        $dbType = $this->getDbTypeFromDriver($this->databaseConfiguration['driver']);
+
+        switch ($dbType) {
+            case 'mysql':
+                return
+                    "CREATE DATABASE `$schemaName`" .
+                    ($charset !== null ? " CHARACTER SET $charset" : '') . /// @todo transform charset name into a supported one
+                    "; CREATE USER '$userName'@'%' IDENTIFIED BY '$password'" .
+                    "; GRANT ALL PRIVILEGES ON `$schemaName`.* TO '$userName'@'%';";
+            case 'pgsql':
+                return
+                    "CREATE DATABASE \"$schemaName\"" . // q: do we need to add 'TEMPLATE template0' ?
+                    ($charset !== null ? " ENCODING $charset" : '') . /// @todo transform charset name into a supported one
+                        "; COMMIT; CREATE USER \"$userName\" WITH PASSWORD '$password'" .
+                        "; GRANT ALL ON DATABASE \"$schemaName\" TO \"$userName\""; // q: should we avoid granting CREATE?
+            default:
+                throw new \OutOfBoundsException("Unsupported database type '$dbType'");
+        }
+    }
+
+    /**
+     * Returns the sql commands used to create a new db schema and accompanying user
+     * @param string $userName
+     * @param string $schemaName
+     * @return string
+     */
+    public function getDropSchemaSQL($userName, $schemaName = null)
+    {
+        if ($schemaName === null) {
+            $schemaName = $userName;
+        }
+
+        $dbType = $this->getDbTypeFromDriver($this->databaseConfiguration['driver']);
+
+        switch ($dbType) {
+            case 'mysql':
+                return
+                    "DROP DATABASE IF EXISTS `$schemaName`; DROP USER '$userName'@'%';";
+            case 'pgsql':
+                return
+                    "DROP DATABASE IF EXISTS \"$schemaName\"; DROP USER IF EXISTS \"$userName\";";
+            default:
+                throw new \OutOfBoundsException("Unsupported database type '$dbType'");
+        }
+    }
+
+    /**
+     * @param string $driver
+     * @return string
+     */
+    protected function getDbTypeFromDriver($driver)
+    {
+        return str_replace('pdo_', '', $driver);
+    }
+}
diff --git a/app/src/Service/SqlExecutor/Forked/Doctrine.php b/app/src/Service/SqlExecutor/Forked/Doctrine.php
@@ -8,7 +8,7 @@
 class Doctrine extends ForkedExecutor implements ForkedCommandExecutor
 {
     /**
-     * @param string $sql
+     * @param string|string[] $sql
      * @return Process
      */
     public function getExecuteCommandProcess($sql)
diff --git a/app/src/Service/SqlExecutor/Forked/NativeClient.php b/app/src/Service/SqlExecutor/Forked/NativeClient.php
@@ -34,7 +34,7 @@ public function getExecuteFileProcess($filename)
      */
     public function getProcess($sqlOrFilename, $isFile = false)
     {
-        $clientType = $this->getDbClientFromDriver($this->databaseConfiguration['driver']);
+        $clientType = $this->getDbClientTypeFromDriver($this->databaseConfiguration['driver']);
 
         switch ($clientType) {
             case 'mysql':
@@ -45,8 +45,10 @@ public function getProcess($sqlOrFilename, $isFile = false)
                     '--user=' . $this->databaseConfiguration['user'],
                     '-p' . $this->databaseConfiguration['password'],
                     '--binary-mode', // 'It also disables all mysql commands except charset and delimiter in non-interactive mode (for input piped to mysql or loaded using the source command)'
-                    // $dbname
                 ];
+                if (isset($this->databaseConfiguration['dbname'])) {
+                    $options[] = $this->databaseConfiguration['dbname'];
+                }
                 if (!$isFile) {
                     $options[] = '--execute=' . $sqlOrFilename;
                 }
@@ -57,14 +59,16 @@ public function getProcess($sqlOrFilename, $isFile = false)
                 break;
             case 'pgsql':
                 $command = 'psql';
+                $connectString = "postgresql://".$this->databaseConfiguration['user'].":".$this->databaseConfiguration['password'].
+                    "@{$this->databaseConfiguration['host']}:".($this->databaseConfiguration['port'] ?? '5432').'/';
+                if (isset($this->databaseConfiguration['dbname'])) {
+                    $connectString .= $this->databaseConfiguration['dbname'];
+                }
                 $options = [
-                    "postgresql://".$this->databaseConfiguration['user'].":".$this->databaseConfiguration['password'].
-                    "@{$this->databaseConfiguration['host']}:".($this->databaseConfiguration['port'] ?? '5432').'/',
-                    //'--host=' . $this->databaseConfiguration['host'],
-                    //'--port=' . $this->databaseConfiguration['port'] ?? '5432',
-                    //'--username=' . $this->databaseConfiguration['user'],
-                    //'--dbname=' . $dbname
+                    $connectString
                 ];
+                // NB: this triggers a different behaviour that piping multiple commands to stdin, namely
+                // it wraps all of the commands in a transaction and allows either sql commands or a single meta-command
                 if (!$isFile) {
                     $options[] = '--command=' . $sqlOrFilename;
                 }
@@ -91,7 +95,7 @@ public function getProcess($sqlOrFilename, $isFile = false)
      * @param string $driver
      * @return string
      */
-    protected function getDbClientFromDriver($driver)
+    protected function getDbClientTypeFromDriver($driver)
     {
         return str_replace('pdo_', '', $driver);
     }
diff --git a/app/src/Service/SqlExecutorFactory.php b/app/src/Service/SqlExecutorFactory.php
@@ -17,7 +17,7 @@ class SqlExecutorFactory
      * @return ForkedCommandExecutor|ForkedFileExecutor
      * @throws \OutOfBoundsException
      */
-    public function createForkedExecutor($databaseConnectionConfiguration, $executionStrategy = 'NativeClient', $timed = true)
+    public function createForkedExecutor(array $databaseConnectionConfiguration, $executionStrategy = 'NativeClient', $timed = true)
     {
         switch ($executionStrategy) {
             case 'Doctrine':

Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@`
`8`	`8`	`class Doctrine extends ForkedExecutor implements ForkedCommandExecutor`
`9`	`9`	`{`
`10`	`10`	`/**`
`11`		`- * @param string $sql`
	`11`	`+ * @param string\|string[] $sql`
`12`	`12`	`* @return Process`
`13`	`13`	`*/`
`14`	`14`	`public function getExecuteCommandProcess($sql)`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ class SqlExecutorFactory`
`17`	`17`	`* @return ForkedCommandExecutor\|ForkedFileExecutor`
`18`	`18`	`* @throws \OutOfBoundsException`
`19`	`19`	`*/`
`20`		`- public function createForkedExecutor($databaseConnectionConfiguration, $executionStrategy = 'NativeClient', $timed = true)`
	`20`	`+ public function createForkedExecutor(array $databaseConnectionConfiguration, $executionStrategy = 'NativeClient', $timed = true)`
`21`	`21`	`{`
`22`	`22`	`switch ($executionStrategy) {`
`23`	`23`	`case 'Doctrine':`