From 207f94095abdd7be874989f1ee29f6fd6807850c Mon Sep 17 00:00:00 2001 From: Pieter Frenssen Date: Tue, 9 Oct 2018 23:47:35 +0300 Subject: [PATCH 1/2] OPENEUROPA-1271: Test that it is possible to specify the --skip-permissions-setup option in configuration. --- tests/AbstractTest.php | 1 + tests/Commands/DrupalCommandsTest.php | 32 +++++++++++++++---- .../fixtures/commands/drupal-site-install.yml | 21 ++++++++++++ 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/tests/AbstractTest.php b/tests/AbstractTest.php index ef3134ad..4884c621 100644 --- a/tests/AbstractTest.php +++ b/tests/AbstractTest.php @@ -20,6 +20,7 @@ abstract class AbstractTest extends TestCase protected function setUp() { $filesystem = new Filesystem(); + $filesystem->chmod($this->getSandboxRoot(), 0777, umask(), true); $filesystem->remove(glob($this->getSandboxRoot()."/*")); date_default_timezone_set('Europe/London'); } diff --git a/tests/Commands/DrupalCommandsTest.php b/tests/Commands/DrupalCommandsTest.php index 1ca8fd33..30beb9ae 100644 --- a/tests/Commands/DrupalCommandsTest.php +++ b/tests/Commands/DrupalCommandsTest.php @@ -17,10 +17,22 @@ class DrupalCommandsTest extends AbstractTest { /** * @param array $config + * The configuration array to pass to the command. + * @param string $command + * The command to execute. + * @param bool $expected_error + * Whether or not it is expected that the command will return an error + * code. + * @param string $expected_settings_dir_permission + * A string representing the octal permission number that is expected to + * be applied on the settings folder. + * @param string $expected_settings_file_permission + * A string representing the octal permission number that is expected to + * be applied on the settings file. * * @dataProvider drupalSettingsDataProvider */ - public function testPermissions(array $config) + public function testPermissions(array $config, $command, $expected_error, $expected_settings_dir_permission, $expected_settings_file_permission) { $configFile = $this->getSandboxFilepath('runner.yml'); file_put_contents($configFile, Yaml::dump($config)); @@ -32,20 +44,28 @@ public function testPermissions(array $config) // Prepare site settings file. $siteSettings = $sitesSubdir . 'settings.php'; touch($siteSettings); - chmod($siteSettings, 0777); + + // Make the settings folder and file unwritable so we can detect whether + // the exception is thrown in `DrupalCommands::validateSiteInstall()` as + // well as whether the permissions are correctly set. + chmod($siteSettings, 0444); + chmod($sitesSubdir, 0555); // Run command. - $input = new StringInput("drupal:permissions-setup --working-dir=" . $this->getSandboxRoot()); + $input = new StringInput("$command --working-dir=" . $this->getSandboxRoot()); $runner = new TaskRunner($input, new BufferedOutput(), $this->getClassLoader()); - $runner->run(); + $exit_code = $runner->run(); + + // Check if an error is returned when this is expected. + $this->assertEquals($expected_error, $exit_code != 0); // Check site directory. $sitesSubdirPermissions = substr(sprintf('%o', fileperms($sitesSubdir)), -4); - $this->assertEquals('0775', $sitesSubdirPermissions); + $this->assertEquals($expected_settings_dir_permission, $sitesSubdirPermissions); // Check site settings file. $siteSettingsPermissions = substr(sprintf('%o', fileperms($siteSettings)), -4); - $this->assertEquals('0664', $siteSettingsPermissions); + $this->assertEquals($expected_settings_file_permission, $siteSettingsPermissions); } /** diff --git a/tests/fixtures/commands/drupal-site-install.yml b/tests/fixtures/commands/drupal-site-install.yml index c19b5397..c768ef95 100644 --- a/tests/fixtures/commands/drupal-site-install.yml +++ b/tests/fixtures/commands/drupal-site-install.yml @@ -1,5 +1,26 @@ +# When the drupal:permissions-setup command is run it should make the settings +# folder and file writable, and there should not be any error. - configuration: drupal: settings: sites-subdir: "build/sites/default/" settings_file: "settings.php" + command: "drupal:permissions-setup" + expected_error: false + expected_settings_dir_permission: "0775" + expected_settings_file_permission: "0664" + +# When the drupal:site-install command is run with the 'skip-permission-setup' +# option set to 'true' then the settings folder and file should not be made +# writable, and an error should be returned. +- configuration: + drupal: + settings: + sites-subdir: "build/sites/default/" + settings_file: "settings.php" + site: + skip_permissions_setup: true + command: "drupal:site-install" + expected_error: true + expected_settings_dir_permission: "0555" + expected_settings_file_permission: "0444" From 65b3b2ce55a4ea4cee628168e70a894a02bcd99a Mon Sep 17 00:00:00 2001 From: Pieter Frenssen Date: Wed, 10 Oct 2018 00:25:53 +0300 Subject: [PATCH 2/2] OPENEUROPA-1271: Allow valueless options to be used in configuration files. --- config/commands/drupal.yml | 2 - src/Commands/AbstractCommands.php | 62 ++++++++++++++++++++++++++++++- src/Commands/DrupalCommands.php | 49 +++++++++++++++--------- 3 files changed, 92 insertions(+), 21 deletions(-) diff --git a/config/commands/drupal.yml b/config/commands/drupal.yml index b2be87f8..8a3ec2b8 100644 --- a/config/commands/drupal.yml +++ b/config/commands/drupal.yml @@ -24,7 +24,6 @@ command: database-password: ${drupal.database.password} sites-subdir: ${drupal.site.sites_subdir} config-dir: ${drupal.site.config_dir} - skip-permissions-setup: ${drupal.site.skip_permissions_setup} drush-setup: options: config-dir: ${drupal.root}/drush @@ -33,7 +32,6 @@ command: sites-subdir: ${drupal.site.sites_subdir} settings-override-file: ${drupal.site.settings_override_file} force: ${drupal.site.force} - skip-permissions-setup: ${drupal.site.skip_permissions_setup} permissions-setup: options: sites-subdir: ${drupal.site.sites_subdir} diff --git a/src/Commands/AbstractCommands.php b/src/Commands/AbstractCommands.php index 3bc04352..123c7d6c 100644 --- a/src/Commands/AbstractCommands.php +++ b/src/Commands/AbstractCommands.php @@ -2,15 +2,17 @@ namespace OpenEuropa\TaskRunner\Commands; +use Consolidation\AnnotatedCommand\AnnotationData; use Robo\Common\ConfigAwareTrait; use Robo\Common\IO; +use Robo\Contract\BuilderAwareInterface; use Robo\Contract\ConfigAwareInterface; use Robo\Contract\IOAwareInterface; -use Robo\Contract\BuilderAwareInterface; use Robo\Exception\TaskException; use Robo\LoadAllTasks; use Robo\Robo; use Symfony\Component\Console\Event\ConsoleCommandEvent; +use Symfony\Component\Console\Input\InputInterface; /** * Class AbstractCommands @@ -47,6 +49,64 @@ public function initializeRuntimeConfiguration(ConsoleCommandEvent $event) Robo::loadConfiguration([$this->getConfigurationFile()], $this->getConfig()); } + /** + * Returns an array of valueless options and their corresponding config key. + * + * This provides the data for ::initializeValuelessOptions(). + * + * @see \OpenEuropa\TaskRunner\Commands\AbstractCommands::initializeValuelessOptions() + * + * @return array + * An associative array, keyed by command name, with each value consisting + * of an associative array where the key is the valueless option name, and + * the value is the location where the option should be placed in the + * configuration array. Example: + * 'drupal:my-command' => ['my-option' => 'drupal.site.my_option'] + */ + public function getValuelessConfigurationKeys() + { + return []; + } + + /** + * Initializes valueless options. + * + * Valueless options (i.e. options of type `InputOption::VALUE_NONE`) cannot + * be defined in the usual way in a YAML configuration file such as + * `drupal.yml` since the library which is responsible for value + * substitution in config files only does straight string replacement. It + * does not support the notion that the absence of a valueless option means + * that it should be set to `FALSE`. + * + * This method uses the data from `::getValuelessConfigurationKeys()` to + * look up whether the option is present in the specified location in the + * configuration files, and will set the value to FALSE if the option is + * missing. If the option is present it will use its actual value. + * + * If the valueless option is passed on the command line this will take + * precedence over the value in the configuration files. + * + * @hook init * + */ + public function initializeValuelessOptions(InputInterface $input, AnnotationData $annotationData) + { + $command_name = $annotationData->get('command'); + $keys = $this->getValuelessConfigurationKeys(); + if (!array_key_exists($command_name, $keys)) { + return; + } + foreach ($keys[$command_name] as $option => $key) { + // Check if the option was passed on the command line. This takes + // precedence over the presence of the value in the configuration + // file. + if (!$input->getOption($option)) { + // If not, take the corresponding value from the configuration. + $value = (bool) $this->getConfig()->get($key); + $input->setOption($option, $value); + } + } + } + /** * @param string $name * @return string diff --git a/src/Commands/DrupalCommands.php b/src/Commands/DrupalCommands.php index 35525296..2de01a37 100644 --- a/src/Commands/DrupalCommands.php +++ b/src/Commands/DrupalCommands.php @@ -32,6 +32,18 @@ public function getConfigurationFile() return __DIR__.'/../../config/commands/drupal.yml'; } + /** + * {@inheritdoc} + */ + public function getValuelessConfigurationKeys() + { + return [ + 'drupal:site-install' => [ + 'skip-permissions-setup' => 'drupal.site.skip_permissions_setup', + ], + ]; + } + /** * Set runtime configuration values. * @@ -87,24 +99,25 @@ public function validateSiteInstall(CommandData $commandData) * * @command drupal:site-install * - * @option root Drupal root. - * @option site-name Site name. - * @option site-mail Site mail. - * @option site-profile Installation profile - * @option site-update Whereas to enable the update module or not. - * @option site-locale Default site locale. - * @option account-name Admin account name. - * @option account-password Admin account password. - * @option account-mail Admin email. - * @option database-type Deprecated, use "database-scheme" - * @option database-scheme Database scheme. - * @option database-host Database host. - * @option database-port Database port. - * @option database-name Database name. - * @option database-user Database username. - * @option database-password Database password. - * @option sites-subdir Sites sub-directory. - * @option config-dir Config export directory. + * @option root Drupal root. + * @option site-name Site name. + * @option site-mail Site mail. + * @option site-profile Installation profile + * @option site-update Whereas to enable the update module or not. + * @option site-locale Default site locale. + * @option account-name Admin account name. + * @option account-password Admin account password. + * @option account-mail Admin email. + * @option database-type Deprecated, use "database-scheme" + * @option database-scheme Database scheme. + * @option database-host Database host. + * @option database-port Database port. + * @option database-name Database name. + * @option database-user Database username. + * @option database-password Database password. + * @option sites-subdir Sites sub-directory. + * @option config-dir Config export directory. + * @option skip-permissions-setup Whether to skip making the settings file and folder writable during installation. * * @aliases drupal:si,dsi *