Create a site UUID on install and only allow config sync between sites with the same UUID

So yes, #2127573-13: Stop creating node, comment and custom block fields automatically and provide defaults in CMI details some nasty consequences of trying to support "config sync between any and all random drupal installs" (i.e make sure that the install process produces the same UUIDs for the same config entities each and every time).

I've been thinking too that we should only reasonably support "config sync between *copies* of a single initial install" - which is the original promise of CMI: deploy between dev / stage / prod. Then, no more problems of mismatching UUIDs since they have all been created only once in the intial install and then copied:
- no need to hardcode UUIDs in config entities shipped in modules/profiles default config
- no need to go through hoops to forbid programmatic creation of config entities in hook_install()
- no need to go through hoops to prevent runtime code from performing secondary config writes when importing a config entity from the "default config" (i.e "set the isSyncing() flag during the 'default config import' process" - see #2127573-23: Stop creating node, comment and custom block fields automatically and provide defaults in CMI)

Similarly, I've been trying to figure out the drawbacks of this approach too. Here's what I found.

1) 6 months after site launch, you welcome a new dev on the team. The only way to set up a new local dev instance is to copy "code + active config folder + db" from prod - which might be a huge db. No way to start fresh on an empty site with "just" the config, since you cannot run the installer, and so nothing will initialize your local db.

Discussing that with @amateescu last week, we figured that maybe you could export the db *schema* from prod, without the data.
"Empty site" = the code + the config ready in the /active folder + a db with empty tables, here you go ?
If we look at current core, that might be pretty close (problem of users 0 and 1 though), but it's difficult to evaluate whether it will still be true in all of contrib ?

2) At least some CMI files shipped in default config will still contain UUIDs, since most contrib devs will fill the config/ folder of their module by picking and copying from their active config.
What should we do in that case ?
- Treat those as "hardcoded UUIDs" ? (would be really inconsistent, some default config ship with hardcoded UUIDs, some other get UUIDs that will vary site by site...=
- Just overlook the UUID key if present when importing default config ? (would be misleading / confusing ?)
- Die with an exception ? CMI files copied from the active store need to be manually cleaned of their UUID key before they can be shipped in a "default config" folder ?

(side note: Field API "field" and "instance" records might be problematic here, since the instance currently refers to its associated field by a 'field_uuid' property in the CMI file, so shipping those in default config assumes the field has a fixed, known UUID that the instance can point at. The recent cleanups in Field API might let us get rid of that 'field_uuid' entry in instance records, and have them refer to their field just by 'field_name'. Not currently the case though)

3) There is still an issue of race conditions during "default config" import.
Example (same as in #2127573-13: Stop creating node, comment and custom block fields automatically and provide defaults in CMI):
- My foobar.module does things on nodes per node type. It stores the corresponding settings in foobar.settings.[node_type].yml.
Typically, it implements hook_entity_node_type_save() to save a new, default, foobar.settings.[node_type] ConfigEntity whenever a new node type is created.
- I install some barbaz.module, that happens to ship a node.type.barbaz.yml in its config/ folder.
It *may* also know about my foobar.module and ship the corresponding foobar.settings.barbaz.yml as well, with fine-tuned values of its own.

What happens then ? In the other issue I pointed the consequences of setting the isSyncing() flag during "default config import" process, to prevent foobar.module from creating its foobar.settings.barbaz while we're importing barbaz/config/node.type.barbaz.yml: this results in a site with a collection of node types, and only some of them having corresponding foobar.settings.[node_type] records. Foobar module, and all others doing similar stuff, needs to account for that uncertainty by moving away from entity_load() and coming up with their own wrapper APIs with an if(exists) check.

So let's assume here we that don't try to do that: when importing barbaz/config/node.type.barbaz.yml, foobar_entity_node_type_save() runs normally and (tries to) create the foobar.settings.barbaz ConfigEntity. 2 cases, depending on import order (currently based on alphabetical order - meaning both will happen, depending on module names):
a) barbaz/config/foobar.settings.barbaz.yml has already been imported earlier in the process so foobar.settings.barbaz already exists - not sure what happens. I think there is an issue/patch somewhere to raise an exception when saving a new config entity with a name (ID) that already exists.
--> foobar_entity_node_type_save() probably needs to wrap some if (!exists) {create 'foobar.settings.barbaz'} logic.
b) the "default" foobar.settings.barbaz gets created ok, then the import process moves along and tries to import barbaz/config/foobar.settings.barbaz.yml: it already exists, not sure what happens either.

(The desired outcome, of course, is that, whatever the module names and alphabet order, active config ends up with the foobar.settings.barbaz that was present in barbaz/config)

I do think this is solvable, but it needs a bit of thinking ;-)

Side note: Drawback 1) is what #1613424: [META] Allow a site to be installed from existing configuration is about.

A "Site UUID" is essentially the purpose of this existing variable in settings.php:

$drupal_hash_salt = '';

Of course, one could argue whether a system.site:uuid configuration value might be cleaner. But in theory and practice, $drupal_hash_salt is used for the identical purpose, both in core (session handling, access tokens, anonymous Update module site identification for d.o, etc) as well as contributed modules.

I think if we're going to replace $drupal_hash_salt with a system.site:uuid configuration value, then we should do so globally (removing $drupal_hash_salt as a result).

Lastly, @yched's #2 2) and 3) are very interesting thoughts, but appear to be off-topic for this issue? Would you mind to move those considerations to #2121751: [META] Making configuration synchronisation work or some more focused issue? :)

They're not really off topic, they're consequences of *this* approach to the "syncing / config entities UUIDs" problem (as opposed to the other approaches that were discussed in #2127573: Stop creating node, comment and custom block fields automatically and provide defaults in CMI and #2138559: Replace UUIDs in configuration entities with a creation ID, which had different consequences). Detailing them was part of the exploration for each approach.

Pulling in configuration from another site is called a migration not sync. I suspect it won't be a sync; it will just override crudely but it will essentially work.

I *think* what people intend when they want to "sync" with config of a different site is to get "features" ported over. Packages of config, like a content type with a view, etc. prototyped on a different site to be added to another. If that is indeed the purpose of this wish, then I think that would indeed be solved different from the sync process. Supporting partially reusing components of the site where this "feature" is being imported to (eg. an existing content type) sounds like a possible but twisted use case to support.

Note: #1 detailed three caveats with the "only support config sync within a ring of sites that share the same site UUID" appraoch.

Caveat 1. (you can only add a new localdev site to the ring by copying the code + config + full db from prod) is solved by #1613424: [META] Allow a site to be installed from existing configuration.

Caveats 2. (what to do with "default config" files that contain leftover UUIDs)
and 3. (race conditions on "default config" import)
are still up there though.

Do we want to open separate issues ?

(also, this issue here about the exact scope of the whole "config sync" feature is IMO an important part of what makes #1613424: [META] Allow a site to be installed from existing configuration critical. Shouldn't the issue summary over there point back to this one here ?)

[edit: removed, I was on crack]

re: caveat 2+3: #2161149: Force-create new UUIDs in default config objects during initial import/creation

Polished the code and improved the UI error message:

The staged configuration cannot be imported, because it originates from a different site than this site. You can only synchronize configuration between cloned instances of this site.

Sorry for the unnecessarily large interdiff, but I think the uuid value should really come first in system.site and everywhere else, as it's going to be the most important property.

I like like this patch, too.

Here's a re-roll, plus a minor language tweak.

Patch looks good.

Only uber-nitpicky remark:

1. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php
   @@ -116,6 +116,23 @@ function testImportLock() {
   +    // Verify that there are configuration differences to import.
   +    $this->drupalGet('admin/config/development/configuration');
   +    $this->assertText(t('The staged configuration cannot be imported, because it originates from a different site than this site. You can only synchronize configuration between cloned instances of this site.'));
   +    $this->assertNoFieldById('edit-submit', t('Import all'));
   

   Comment looks off ?

Other than that, I just feel that, since this is a fairly important and defining restriction we put on the config sync process here, we miss a place where we actually capture that restriction and a "short" version of the reasoning that led there *in comments in the code*, rather than buried in the issue queue.

Proposal:
The config sync process relies on uniqueness of the config entities' UUIDs. It therefore only supports deployment between site instances that share the same "site UUID", which indicates that they have been created by cloning off from one single initial install (e.g. "dev / stage / prod"). Deploying configuration between any two random drupal installs is not supported.

Just not sure where this would best belong :-)
Also, not a blocker. This is RTBC IMO.

Side note (not for this issue, of course) :

--- a/core/lib/Drupal/Core/Config/ConfigImporter.php
+++ b/core/lib/Drupal/Core/Config/ConfigImporter.php

In our series about "Config Sync and Default Config Import are in fact two separate and fairly different processes", we'd gain a bit of clarity if the class that does "sync" was not called "importer" ;-)

The attached patch should fix the fails (introduced by #2012566: Naming confusion between config_comparer and storage_comparer in config.admin.inc and other places)

Hm. I tried to test this patch with my handy CMI testy script at https://github.com/webchickenator/drupal8-demo/blob/master/cmi.sh but unfortunately something has caused that to seriously b0rk in the past few weeks. :( So I'll just take it on faith that the folks in this issue have tested some deployments manually with this patch and have confirmed that they are working properly.

Committed and pushed to 8.x. Thanks!

I updated the "Example" section of the documentation on https://drupal.org/documentation/administer/config to reflect this new requirement, and note the target workflow described in the meta issue.

Moved the race condition issue described in #2.3 over at #2170579: Race conditions on "default config import"

alexpott suggested a couple enhancements which would make this validation less painful:

1. Add a way to break the site UUID validation and allow the user to import anyway.
2. or an initial sync process that assumes all matching config id should have the same UUID and does the appropriate thing.