From 0618a882754474f6f29dd8f8c93deab49f4c5a0c Mon Sep 17 00:00:00 2001 From: Dane Powell Date: Thu, 20 Apr 2017 11:34:15 -0700 Subject: [PATCH 1/5] Added documentation on protecting against configuration corruption. --- readme/configuration-management.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/readme/configuration-management.md b/readme/configuration-management.md index 8a4edbc6d..558fda119 100644 --- a/readme/configuration-management.md +++ b/readme/configuration-management.md @@ -23,7 +23,7 @@ Generally speaking, a configuration change follows this lifecycle: 4. Automated testing ensures that the configuration can be installed from scratch on a new site as well as imported without conflicts on an existing site. 5. After the change is deployed, deployment hooks automatically import the new or updated configuration. -The way that configuration is captured and deployed between environments in Drupal 8 is typically via YAML files. These YAML files, typically stored in a root `config` directory, or distributed with individual modules in `config/install` directories, represent individual configuration objects that can be synchronized with the active configuration in an environment's database via a variety of methods. See [documentation on core configuration management](https://www.drupal.org/docs/8/configuration-management). +The way that configuration is captured and deployed between environments in Drupal 8 is typically via YAML files. These YAML files, typically stored in a root `config` directory, or distributed with individual modules in `config/install` directories, represent individual configuration objects that can be synchronized with the active configuration in an environment's database via a variety of methods. See [documentation on core configuration management](https://www.drupal.org/docs/8/configuration-management). This document address the challenge of capturing ("exporting") and deploying ("importing") configuration in a consistent way in order to support the workflow described above. @@ -68,6 +68,16 @@ We need to find a better way of preventing this than manually monitoring module * [Features and contributed module updates](https://www.drupal.org/node/2745685) * [Testing for schema changes to stored config](https://github.com/acquia/blt/issues/842). +### Ensuring integrity of stored configuration + +Configuration stored on disk, whether via the core configuration system or features, is essentially a flat-file database and must be treated as such. For instance, all changes to configuration should be made via the UI or an appopriate API and then exported to disk. You should never make changes to individual config files by hand, just as you would never write a raw SQL query to add a Drupal content type. Even seemingly small changes to one part of the configuration can have sweeping and unanticipated changes. For instance, enabling the Panelizer or Workbench modules will modify the configuration of every content type on the site. + +BLT has a built-in test that will help protect against some of these mistakes. After configuration is imported (i.e. during `local:update` or `deploy:update`), it will check if any configuration remains overridden. If so, the build will fail, alerting you to the fact that there are uncaptured configuration changes or possibly a corrupt configuration export. This test acts as a canary and should not be disabled, but if you need to temporarily disable it in an emergency (i.e. if deploys to a cloud environment are failing), you can do so by settings `cm.allow-overrides` to `true`. + +Finally, you should enable protected branches in Github to ensure that pull requests can only be merged if they are up to date with the target branch. This protects against a scenario where, for instance, one PR adds a new content type, while another PR enables Workbench (which would modify that content type). Individually, each of these PRs is perfectly valid, but once they are both merged they produce a corrupt configuration (where the new content type is lacking Workbench configuration). When used with BLT’s built-in test for configuration overrides, protected branches can quite effectively prevent some forms of configuration corruption. + +For an ongoing discussion of how to ensure configuration integrity, see https://www.drupal.org/node/2869910 + ## Configuration Split workflow ### Overview From 304904d05fee089fa37d992f7ad74cf5fae2952c Mon Sep 17 00:00:00 2001 From: Dane Powell Date: Fri, 21 Apr 2017 15:16:09 -0700 Subject: [PATCH 2/5] Added config override check. --- src/Robo/Commands/Setup/ConfigCommand.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Robo/Commands/Setup/ConfigCommand.php b/src/Robo/Commands/Setup/ConfigCommand.php index 57ecc5264..8d1fc0a52 100644 --- a/src/Robo/Commands/Setup/ConfigCommand.php +++ b/src/Robo/Commands/Setup/ConfigCommand.php @@ -66,6 +66,13 @@ public function import() { $task->exec("drush @$drush_alias pm-enable config_split --yes"); $task->exec("drush @$drush_alias config-import sync --yes"); } + if (!$this->getConfigValue('cm.allow-overrides')) { + $this->say("Checking for config overrides..."); + $config_overrides = $task->exec("drush @$drush_alias cex sync -n | grep 'Differences of the active config'"); + if ($config_overrides) { + throw new \Exception("Configuration in the database does not match configuration on disk. You must re-export configuration to capture the changes. This could also indicate a problem with the import process, such as changed field storage for a field with existing content."); + } + } break; case 'features': From 0930c24095ce10e9ff66a41d4bbcf1cfead5e4dd Mon Sep 17 00:00:00 2001 From: Dane Powell Date: Thu, 27 Apr 2017 10:59:45 -0700 Subject: [PATCH 3/5] Fixed robo command. --- src/Robo/Commands/Setup/ConfigCommand.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Robo/Commands/Setup/ConfigCommand.php b/src/Robo/Commands/Setup/ConfigCommand.php index 527a29a9b..ca650249a 100644 --- a/src/Robo/Commands/Setup/ConfigCommand.php +++ b/src/Robo/Commands/Setup/ConfigCommand.php @@ -102,8 +102,8 @@ protected function importConfigSplit($task, $drush_alias) { } if (!$this->getConfigValue('cm.allow-overrides')) { $this->say("Checking for config overrides..."); - $config_overrides = $task->exec("drush @$drush_alias cex sync -n | grep 'Differences of the active config'"); - if ($config_overrides) { + $config_overrides = $this->taskExec("drush @$drush_alias cex sync -n | grep 'active configuration is identical'"); + if (!$config_overrides->run()->wasSuccessful()) { throw new \Exception("Configuration in the database does not match configuration on disk. You must re-export configuration to capture the changes. This could also indicate a problem with the import process, such as changed field storage for a field with existing content."); } } From 98cb0f94c0eaac5da5cea5c026edc7761d4d666c Mon Sep 17 00:00:00 2001 From: Dane Powell Date: Thu, 27 Apr 2017 11:33:02 -0700 Subject: [PATCH 4/5] Moved task execution. --- src/Robo/Commands/Setup/ConfigCommand.php | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/Robo/Commands/Setup/ConfigCommand.php b/src/Robo/Commands/Setup/ConfigCommand.php index ca650249a..56a104102 100644 --- a/src/Robo/Commands/Setup/ConfigCommand.php +++ b/src/Robo/Commands/Setup/ConfigCommand.php @@ -67,6 +67,16 @@ public function import() { $task->exec("drush @$drush_alias cache-rebuild"); $task->run(); + // Check for configuration overrides. + if (!$this->getConfigValue('cm.allow-overrides')) { + $this->say("Checking for config overrides..."); + $config_overrides = $this->taskExec("drush @$drush_alias cex sync -n | grep 'active configuration is identical'"); + $config_overrides->dir($this->getConfigValue('docroot')); + if (!$config_overrides->run()->wasSuccessful()) { + throw new \Exception("Configuration in the database does not match configuration on disk. You must re-export configuration to capture the changes. This could also indicate a problem with the import process, such as changed field storage for a field with existing content."); + } + } + $this->invokeHook('post-config-import'); } @@ -100,13 +110,6 @@ protected function importConfigSplit($task, $drush_alias) { $task->exec("drush @$drush_alias pm-enable config_split --yes"); $task->exec("drush @$drush_alias config-import sync --yes"); } - if (!$this->getConfigValue('cm.allow-overrides')) { - $this->say("Checking for config overrides..."); - $config_overrides = $this->taskExec("drush @$drush_alias cex sync -n | grep 'active configuration is identical'"); - if (!$config_overrides->run()->wasSuccessful()) { - throw new \Exception("Configuration in the database does not match configuration on disk. You must re-export configuration to capture the changes. This could also indicate a problem with the import process, such as changed field storage for a field with existing content."); - } - } } /** From b21b3449898463428eaed3059f3eaf7725bde548 Mon Sep 17 00:00:00 2001 From: Dane Powell Date: Thu, 27 Apr 2017 11:51:02 -0700 Subject: [PATCH 5/5] Fixing travis. --- src/Robo/Commands/Setup/ConfigCommand.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Robo/Commands/Setup/ConfigCommand.php b/src/Robo/Commands/Setup/ConfigCommand.php index 56a104102..b2fe06eb7 100644 --- a/src/Robo/Commands/Setup/ConfigCommand.php +++ b/src/Robo/Commands/Setup/ConfigCommand.php @@ -70,7 +70,7 @@ public function import() { // Check for configuration overrides. if (!$this->getConfigValue('cm.allow-overrides')) { $this->say("Checking for config overrides..."); - $config_overrides = $this->taskExec("drush @$drush_alias cex sync -n | grep 'active configuration is identical'"); + $config_overrides = $this->taskExec("drush @$drush_alias cex sync -n"); $config_overrides->dir($this->getConfigValue('docroot')); if (!$config_overrides->run()->wasSuccessful()) { throw new \Exception("Configuration in the database does not match configuration on disk. You must re-export configuration to capture the changes. This could also indicate a problem with the import process, such as changed field storage for a field with existing content.");