BLT 8.9.x + Drush 8.1.x results doesn't leverage config storage correctly during installations

[amcgowanca]

Currently, I'm working on a multisite build where each site is sharing similar configurations and then enhanced through config-split for site specializations. Part of the site-specializations include certain languages installed, however, the translated config objects from Core are within the sync directory as various collections (/config/default/language/*). All has been working as expected until recently, where we have began leveraging config-split more so. I've recently began to encounter an issue where languages are being installed as expected through the initial Drupal install tasks which are active within a split, where being deleted in Drush's secondary config-import operations. In addition, what appears to be odd is that it this was a continuous hit-or-miss as to when or why it would happen but has been growing rapidly to a point where I've needed to dig into it.

However, while I have not explicitly narrowed down the hit-or-miss reasons... I've noticed that Drush 8.1.x uses the FileStorage class explicitly when the --config-dir option is set during the site-install command as a result of the source argument not existing. The problem here is that configuration collections for languages are not be recognized correctly during the site-install as the FilteredStorage service from the config_filter module (and therefore its usage needed for config_split) is no longer decorating the config.storage.sync as it would normally when using the storage service as, \Drupal::service('config.storage.sync'); and therefore presenting some unexpected behaviours.

Looping back to BLT... I'm trying to explicitly determine if this is in fact a case where BLT 8.9.x needs to solve for or if it is just a use case for this particular project I'm working on. Specifically, during the site-install operation, BLT is specifying the --config-dir option when the cm.strategy config value is not equal to none. This then results in Drush using the FileStorage class explicitly for config importing instead of Drupal's sync/staging storage service: config.storage.sync. As a result, when we enhance this with Config Split and the Config Filter, the secondary import doesn't actually make use of config filter's filtered storage decorator and therefore this results in any config that is explicit to the split (filter) being deleted until we explicitly specify the actual key as "sync" during the import later on.

Suggestion would be to ultimately revise the following lines of code:

    // --config-dir is not valid for Drush 9.
    if ($config_strategy != 'none' && $this->getInspector()->getDrushMajorVersion() == 8) {
      $cm_core_key = $this->getConfigValue('cm.core.key');
      $task->option('config-dir', $this->getConfigValue("cm.core.dirs.$cm_core_key.path"));
    }

in DrupalCommand::install(), so that we are:

1. Not performing a site-install with --config-dir
   2.a) Explicitly set the site's UUID based on the value stored in the sync directory. The reason for this is that the ConfigImporter and storage comparer usages for import in Drupal core will validate the UUID of system.site.yml against that of the active storage.
   2.b) Explicitly performing a drush config-import similar to that of the blt setup:config-import where we are using the cm.core.key value as the "source" argument to the command.

[danepowell]

In the latest version of BLT (9.1.x), we have completely removed the --config-dir parameter: #2080

Instead, we recommend that you use the new core patch to install sites from existing config, which takes Drush and BLT entirely out of the equation for the initial config import: https://github.com/acquia/blt/wiki/Drush-9#installing-site-from-existing-configuration

Does this solve the issue for you? Either way, I'm not sure that BLT could do anything differently since we've already stopped using --config-dir.

[amcgowanca]

@danepowell ,

I think its simply that we have not yet upgrades to Drush 9 and BLT 9.x on this particular project. The primary reason is due to ACSF related requirements and multiple specializations completed for 8.9.10 that I've had to implement.

That said, ... This is how I'd solve it in 8.9.x versions:

1. Perform site installation without --config-dir. This would, I believe, impose that config is installed via Drupal as expected and therefore via the patches to core installing based on config/*.
2. Perform a secondary set of the system.site uuid value based on that stored in system.site.yml.
3. Run an explicit import of Drush's config-import (noting this is applicable in Drush 8.1, haven't looked at Drush 9 yet) where the source argument is set to sync.

Though... I'm not sure if this would then result in problems related to multisite/ACSF users that are using custom storage directories for config that do not match that of the specified SYNC directory.

... Sample of the method that I implemented in this particular project:

<?php

namespace Acquia\Blt\Custom\Commands;

use Acquia\Blt\Robo\Commands\Setup\DrupalCommand;
use Acquia\Blt\Robo\Common\RandomString;
use Acquia\Blt\Robo\Exceptions\BltException;

/**
 * Specialized Drupal setup related commands such as installation.
 */
class CustomSetupDrupalCommand extends DrupalCommand {

  /**
   * Defines the site UUID value.
   *
   * Due to each site's system.site.yml having the same UUID value as a result
   * of the system.site.yml specifically originating in the config.storage.sync,
   * instead of reading this value - we will just hardcode it for now.
   */
  const SITE_UUID = '2df146b4-b1a5-11e7-abc4-cec278b6b50a';

  /**
   * Overrides Acquia BLT's internal Drupal install.
   *
   * @hook replace-command internal:drupal:install
   *
   * @validateMySqlAvailable
   *
   * @see https://github.com/acquia/blt/issues/2297
   */
  public function install() {
    /** @var \Acquia\Blt\Robo\Tasks\DrushTask $task */
    $task = $this->taskDrush()
      ->drush("site-install")
      ->arg($this->getConfigValue('project.profile.name'))
      ->rawArg("install_configure_form.update_status_module='array(FALSE,FALSE)'")
      ->rawArg("install_configure_form.enable_update_status_module=NULL")
      ->option('site-name', $this->getConfigValue('project.human_name'))
      ->option('site-mail', $this->getConfigValue('drupal.account.mail'))
      ->option('account-name', $this->generateRandomUsernameString(), '=')
      ->option('account-mail', $this->getConfigValue('drupal.account.mail'))
      ->option('locale', $this->getConfigValue('drupal.locale'))
      ->verbose(TRUE)
      ->assume(TRUE)
      ->printOutput(TRUE);
    $result = $task->detectInteractive()->run();
    if (!$result->wasSuccessful()) {
      throw new BltException("Failed to install Drupal!");
    }
    $this->getConfig()->set('state.drupal.installed', TRUE);
    $config_strategy = $this->getConfigValue('cm.strategy');
    if ($config_strategy != 'none' && $this->getInspector()->getDrushMajorVersion() == 8) {
      $result = $this->taskDrush()
        ->assume(TRUE)
        ->verbose(TRUE)
        ->drush('config-set')
        ->args(['system.site', 'uuid', self::SITE_UUID])
        ->run();
      if (!$result->wasSuccessful()) {
        throw new BltException("Failed to set Drupal's system.site uuid value.");
      }
      $cm_core_key = $this->getConfigValue('cm.core.key');
      $result = $this->taskDrush()
        ->assume(TRUE)
        ->verbose(TRUE)
        ->drush('config-import')
        ->arg($cm_core_key)
        ->run();
      if (!$result->wasSuccessful()) {
        throw new BltException("Failed to perform initial site-install with config-import specifying source as: '$cm_core_key'.");
      }
    }
    return $result;
  }

  /**
   * Generates a random, valid, username.
   *
   * @param int $length
   *   The number of characters the username should be in length.
   *
   * @return string
   *   The username.
   *
   * @see \Drupal\user\Plugin\Validation\Constraint\UserNameConstraintValidator
   */
  protected function generateRandomUsernameString($length = 10) {
    return RandomString::string($length, FALSE,
      function ($string) {
        return !preg_match('/[^\x{80}-\x{F7} a-z0-9@+_.\'-]/i', $string);
      },
      'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!#%^&*()_?/.,+=><'
    );
  }
}

[danepowell]

You actually don't need Drush 9 in order be able to install sites from existing config natively (without Drush's config-dir argument or any sort of monkeying with UUIDs). Just follow the steps here: https://github.com/acquia/blt/wiki/Drush-9#installing-site-from-existing-configuration

That page describes it in the context of Drush 9 but Drush 9 is not required.

BLT and Lightning have both committed to this approach for config imports, it is much more reliable than any other method, including using --config-dir or rolling your own installer/importer. Please try it and let me know if it works for you.

In general, any method of installing Drupal that imports module-provided configuration (i.e. a normal Drupal install) and then tries to clobber that installed configuration afterwards by running a config-import is going to cause trouble because of UUID conflicts. It's much better to circumvent Drupal's native install behavior (via that patch) and just import all default configuration at once, at the site level.

[danepowell]

Closing due to inactivity... as I mentioned above I think there better ways to solve this problem that aren't specific to BLT.