Do not ship with default UUIDs

+++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
@@ -346,4 +360,15 @@ public function getId() {
+          throw new ConfigImporterException(String::format('Config files @files changed both locally and remotely', array('@files' => implode(', ', $files))));

So my question after reading the issue summary was: "What happens if for a single config object both the local one has changed and there is one to be imported?" (i.e. the actual 3-way merge). This is what happens. Makes sense. Contrib can go crazy if it wants.

Here is a test which at least works when there is a conflict.

I don't understand what this issue is about. We do want the config items that come from default config (either the install profile config or modules config) to have the same UUID on dev and prod.

So yeah, "the frontpage view shipped in standard profile", "the filtered_html input format", "the node body field" will have the same UUID across all drupal sites - I don't see where this is an actual issue ? (+ I don't really welcome the idea of completely redefining what the "import" concept means as a side effect ?)

As I understand it, we are using UUIDs to track changes to configuration, and that default configuration makes this process easier.

In #1969800: Add UUIDs to default configuration we solved an obvious problem for the following situation: 2 sites are installed, nothing changes, config is exported from one, imported in the other, and 50 or more changes are reported.

@chx Could you describe the problem that this issue would fix? In other words, is there some sort of test case for this problem, or is it that we are going to cause problems later because our current architecture is flawed?

Arrrgggh. Convince me this doesn't roll deployments back to being insane. "Cause problems" and "theoretically wrong" are not helpful.

Where was the agreement? I barely opened a conversation when you left.

I'd prefer discussion continued.

I think #1969800: Add UUIDs to default configuration was not the correct solution for the deployment problem. At the very least it's not the only solution. We can find solutions for the problem there in any case so I don't really understand the anger in this issue, quite frankly.

Let's say you install two different sites - dev and prod - and at some point want to deploy the body field on articles from dev to prod. Currently, because they have the same shipped UUID, that's not a problem, you can do it. As #26 correctly notes, though, as soon you delete and re-create the field you can't. So in that case the current solution does not help at all.
So omitting the shipped UUIDs gets you in no worse of a situation than above, you have the same "conceptual" objects with a different UUID.
This problem can be solved manually by simply deleting the body field on prod. Then the one from dev can be imported without problems. So to make that problem easier we can simply do that automatically on config import. If the UUIDs differ then drop the currently active one and import the other one. So that's one of the use-cases that should be considered in #2111199: Decide what to do on import on diverge.

Again, I don't really understand why removing the default UUIDs is such a problem. We need to do some work to make config import usable for all cases in any case.

You have a dev site a and a prod site. You have some fields installed by default. You go on with life, have content on both sites but didnt edit that specific field. You delete the field on dev where the data for that field gets nuked, recreate, import to prod, under the current rules this would be disallowed but under the new rules it would override and the data nuking wouldn't happen. Am I right that this is the problem?

No what should happen is that on prod the previous field would be deleted, and a new one would be recreated in its place. Exactly what happened on dev should happen on prod. That is why we need UUIDs in default config. The new field will have a different UUID, so we know it is a delete and recreate as opposed to a rename (where the UUID would remain the same.) I don't see any reason why either of the scenarios you outline would occur, neither of them makes sense. I am honestly super-confused about this whole issue.

Yes, being able to differenciate between updates and "delete + create different one with same name" is a critical feature of the config impor process, and this is why config entities have UUIDs. This has been a key design pattern for CMI since BADcamp 2011.

As @beejeebus wrote in #25, if this is not backed by uuids this needs to be backed by some other "random non clashing auto-generated hash", but honestly I have no idea why uuids wouldn't do the job.

Also, I don't understand #27. If you delete and recreate the field on dev, it will be deleted and recreated on prod on import, based on UUIDS being different, and that's exactly what we want.

Do we really want to re-hash all this ? :-(

I am currently playing around with configuration import and wanted to join in with a question regarding this issues premise, not to re-hash everything but just to clarify this issues` status since it seems confusing to many.

when i read through the answer on stackexchange it seems like we are in the second situation since we want

to track changes in the wild to this configuration object, and be able to tie them all back together to what the original configuration was or should have been, then what you're really trying to identify is the type, and a pre-generated UUID makes perfect sense.

so are we really misusing uuid fundamentally or not?

if i change the contenttypes and views that come shipped and import those on production they are still the ones that were shipped opposed to if i deleted and recreated them.

The moment you try move configuration between sites having UUID in them will cause problems?

@chx could you elaborate?

Re #28 and #30: I totally agree with everything you said. No, let's not re-hash that, noone is arguing that I think. It's just that currently we support the following use-case, due to the shipped UUIDs:

You install two different sites, one dev, one prod. You then change the default 'node.body' field instances on articles and then want to deploy those changes. Because the prod site has the same UUID it will not delete-recreate but simply update.

Without shipped UUIDs this would not be supported so the question is whether we need to support this use-case. I would say no.

It is worth noting that there is only a practical difference between update and delete-recreate if you already have content in those fields. If you have a completely clean site with no content, it makes no practical difference. (This also applies to most other configuration not only field instances.)

So if you do the following: Install two sites, one dev one prod. And before you do *anything* on prod, you just do a full import from dev. That will get you into the same situation that we currently have, where all your UUIds will match on dev and prod. Therefore I don't think default UUIDs are worthwhile to keep.

Edit: Removed some stray characters.

+++ b/core/modules/forum/config/field.field.forum_container.yml
@@ -1,5 +1,4 @@
-uuid: babf2ba1-505f-4c71-8a07-7be19f4fb9f3

+++ b/core/modules/forum/config/field.instance.taxonomy_term.forums.forum_container.yml
@@ -1,5 +1,4 @@
 field_uuid: babf2ba1-505f-4c71-8a07-7be19f4fb9f3

This patch deletes exactly that which links field instances to fields.

re @tstoeckler:

You install two different sites, one dev, one prod. You then change the default 'node.body' field instances on articles and then want to deploy those changes. Because the prod site has the same UUID it will not delete-recreate but simply update.

Without shipped UUIDs this would not be supported so the question is whether we need to support this use-case. I would say no.

#1969800: Add UUIDs to default configuration was done after the conclusion was "we do need to support this use case - create two fresh installs, one on dev, one on prod, make config changes on dev, deploy on prod".

It is worth noting that there is only a practical difference between update and delete-recreate if you already have content in those fields. If you have a completely clean site with no content, it makes no practical difference. (This also applies to most other configuration not only field instances.)

Not true, there is a practical difference between an update and "delete/create new" even if the field has no data - e.g you can delete and create a different field with same name & a different field type - this is forbidden in the case of an update.

More generally, let's not focus on fields. As been stated many times already in the CMI effort over the the last two years, this is about making a sane & predictable deploy flow for any config entity that triggers different code in its update() and delete() / create() flows - whether in the storage controller, in the entity type class, or in any random hook_entity_OP(). It so happens that in current core, fields are what most spectacularly breaks, but that's not relevant. So let's not reason on "but wait, in the case of fields, this and that work in this and that case if we do this or that", coz contrib will have other cases we don't want to have to predict.

CMI needs to be able to identify whether a file deploy means an update or a delete / create, plain and simple, and independently of the specifics of any specific config entity type. UUIDs in config entities files is how CMI does it.

What might help this conversation move is a test case that lays out the behaviour we're trying to preserve and why, probably using fields as the test case (although I see your point about this not being specific to fields at all). Heck, even a pseudocoded test case would be great. I definitely hear the frustration of the CMI folks who feel like this has been discussed over and over again, but I also hear the confusion of people who think the two Us in UUID ought to mean what they say on the tin. :)

Updated issue summary.

Add a related issue

the conclusion was "we do need to support this use case - create two fresh installs, one on dev, one on prod, make config changes on dev, deploy on prod".

I don't think the fact that we install two separate sites and then retroactively say: "Hey, you're actually just a copy of this other site!" makes much sense, TBH. Still, I think we can get the best of both worlds. We just need to provide an explicit facility to say "Wipe all config of the current site and import from a different one."

Even better: #1613424: [META] Allow a site to be installed from existing configuration would totally fix this in a sane way.

So, right, this is ultimately about "how do you setup two (or more) sites between which you intend to deploy config". As explained above, config deploy can only be expected to work between sites where the "same config entities" have the same UUIDs.

Either:
a) We intend to have "make two separate fresh installs (presumably, using the same install profile), and expect deploying from one to the other just works". In order to do this, config entities created during install need to have the same UUIDs so that they are recognized as "the same" during config import - hence fixed UUIDs in default configs.
b) We state that a) is not supported, and we require more restrictive setup steps: do a fresh install of site A, then setup site B with "some" (TBD) list of exact steps (copy files and db, edit some stuff in settings.php...). Then no need for fixed UUIDs in default config. This also means we do not support importing config between two arbitrary sites (or only creates, not updates)

The problem is that this specific point ("on what kind of setup do we expect config import to work ?") has AFAIK never had a dedicated issue as an important user story / feature definition in CMI, but only discussed / diluted in side issues like this one or #1969800: Add UUIDs to default configuration or #1703168: [Meta] Ensure that configuration system functionality matches expected workflows for users and devs (the continuing vagueness of "expected" being the crux of the issue IMO...). Scope changes during the cycle (initially we allowed partial imports, but we now only support full imports) have also probably muddied the water a bit about what we expect from CMI imports exactly.

My suggestion would be: let's have this discussion once and for all in a dedicated issue ?

We had a discussion about this today at BADCAmp. Basically the conclusion is that it is very very difficult to work through all the different use cases and whether they will work or not without actually having a functioning system of any kind in front of us. So what we really need to do is get CMI to the point where even one basic test case will actually work. One test case we have been discussion for a long time is this

- Install minimal
- Import the config from default
- End up with default

If we can't make this very simple work then we're pretty much done. I think the whole discussion of do we support x workflow or y workflow is pretty dumb when *we cant even test or put any of these workflows in front of users yet*. So we all kind of agree that we just need to push on until we get *something* work that we can play with and try stuff out with. This will install

- Implementing validation in the interest of ...
- Getting module/enable disable on sync working and ...
- Implementing the defer process we discussed at Badcamp two years ago to manage dependency problems

Then we can revisit stuff like this. alexpott is going to create a meta-issue to track this and any other dependencies we have for this one use case.

There is one actual issue that came up around UUIDs in this discussion, and that is the problem of config that is dynamically generated at install-time (and thus can't have a default UUID specified.) I consider this a bug - any default config you need to install should be statically provided by your module. Any places that do this need to be fixed.

There is one actual issue that came up around UUIDs in this discussion, and that is the problem of config that is dynamically generated at install-time (and thus can't have a default UUID specified.) I consider this a bug - any default config you need to install should be statically provided by your module. Any places that do this need to be fixed

Yes, this is a real problem, and the more I think of this, the more I fear it's going to be a can of worms which makes profile/module install time a "special" runtime context where APIs behave in a weird way. More on this when we have a meta issue.

After reading this issue, I think that the original problem statement and the more limited/focused issue title is correct:

Default config that is installed from scratch on a new site needs to get new, universally unique identifiers, in order to actually be universally unique.

Otherwise, foo.bar.yml is going to be the same thing on 500,000 different sites, whereas it is clearly not.

I'm a bit confused by the earlier stated expectation of, when installing two independent sites, and copying the config from site A to site B, then a config import was supposed to simply "merge" everything from site A into site B, disregarding the fact that we're looking at two completely independent sites.

The expected effect of that operation should be that all config entities are deleted and recreated, because they may be similar, but are not the same.

It appears that the precondition in that expectation is the root cause: Instead of installing two different sites, you normally install one site, and you initialize your staging environments by cloning that site into a second (and third) instance. Just like you would have done it prior to D8.

For the special case of initializing a clone based on configuration only, but without content/data (e.g., dev → prod without dev data, or prod → dev for a new developer who doesn't need content/data), I once created #1613424: [META] Allow a site to be installed from existing configuration

The major difference in the operation proposed in #1613424 is that the site-clone is not installed from scratch based on default config (causing new UUIDs), but instead, the site-clone is installed based on a preexisting configuration staging directory (e.g., obtained via git), retaining existing UUIDs.

Therefore, I agree that each default config (entity) that is freshly installed should receive a new UUID when getting installed.

However, there appear to be concerns with that — unfortunately, they were not expressed/elaborated on in a way that allows us to get on the same page.

Can someone clarify what these concerns are and ideally also the related use-cases? Thanks!

@sun: highly confused area currently, spread around many issues :-/
See #2127573-36: Stop creating node, comment and custom block fields automatically and provide defaults in CMI (and the issues linked there) for the current state of the discussion & proposed approaches.

Bumping to critical to match parent task.

@alexpott: do we really want to keep this issue ?
If the plan is to go with #2133325: Create a site UUID on install and only allow config sync between sites with the same UUID, then sure we'll want to get rid of the hardcoded UUIDs in our default config files, but the gist of the discussion and patches here are about yet another mechanism to deal with UUIDs (the "allow UUID change just after install instead" part of the title), which becomes moot if I get things correctly.

So, there are three parts to this issue:

1. Solve the problem default UUIDs were intended to solve -- This is partially done now; with a site UUID, we change the codified expectations so that we no longer allow the scenario that blew up without the default UUIDs. The second half of solving this problem (our alternate solution for that usecase) will come with #1613424: [META] Allow a site to be installed from existing configuration.
2. Actually removing all those painstakingly added UUIDs and the expectation that they'll be there. :P I'll take this on; it's my albatross.
3. Deciding what to do when a UUID is present in default configuration provided by a non-core module. There are three options:
   • Throw an exception.
   • Ignore it.
   • Respect it and install with that UUID.

   @alexpott expressed a preference for throwing an exception. In any case, I think this part of it belongs in a separate issue, and that separate issue is possibly a beta target rather than a beta blocker. I'll take care of filing that as well.

Starter patch strips UUIDs out of default config.

Remaining instances of "uuid" in non-schema yaml files:

[tesla:drupal | Fri 14:06:07] $ grep -r "uuid:" * | grep ".yml" | grep -v ".schema.yml"
core/modules/field/tests/modules/field_test_config/staging/field.instance.entity_test.entity_test.field_test_import_staging.yml:uuid: ea711065-6940-47cd-813d-618f64095481
core/modules/field/tests/modules/field_test_config/staging/field.instance.entity_test.entity_test.field_test_import_staging.yml:field_uuid: 0bf654cc-f14a-4881-b94c-76959e47466b
core/modules/field/tests/modules/field_test_config/staging/field.instance.entity_test.test_bundle.field_test_import_staging_2.yml:uuid: f07794a2-d7cc-45b6-b40d-13cf021b5552
core/modules/field/tests/modules/field_test_config/staging/field.instance.entity_test.test_bundle.field_test_import_staging_2.yml:field_uuid: 2165d9aa-9a0c-41a1-be02-2a49f3405c00
core/modules/field/tests/modules/field_test_config/staging/field.instance.entity_test.test_bundle_2.field_test_import_staging_2.yml:uuid: 49d6dd19-5097-443d-8f00-fc79525bebce
core/modules/field/tests/modules/field_test_config/staging/field.instance.entity_test.test_bundle_2.field_test_import_staging_2.yml:field_uuid: 2165d9aa-9a0c-41a1-be02-2a49f3405c00
core/modules/serialization/serialization.services.yml:  serializer.entity_resolver.uuid:

I think the stuff in field_test_config/staging is not default configuration, but is rather meant to emulate staged configuration. Need to take a closer look at the test to be sure.

The patch also removes two different UUID lines from the default image styles. Something weird is going on there that needs a closer look.

I think the stuff in field_test_config/staging is not default configuration, but is rather meant to emulate staged configuration. Need to take a closer look at the test to be sure.

I think so too, yes.

Ok, this fixes the Field API failures, they need the uuid entries there. I also left in the uuid entry in system.site. It gets a new one when installing and there should be one for the Field API staging tests because the storageComparer checks on it (which probably means that we might need to refactor those tests one day, but still, they work fine).

However, the failure in ConfigSingleImportExportTest() reveals something interesting: config entities managed by the system module are not recognized when the system module is installed: date format and menu config entities do not get a uuid. If you for instance supply a default date format in the node module, that one will get a uuid, which is what we want of course. Additionally, other default config entities which rely on an entity type provided by the module seem to work fine (contact module for instance), so clearing the entity info cache and a new discovery seem to work fine there.

I've attached a second patch with debugging in it which logs data to /tmp/installing which you can use to follow what happens at install.

- edit - the patches here are wrong, use the one in #55 - use the do-not-test for inspiration to log if you want to follow along on install.

Ok, now the right patches.

So yeah, drupal_install_system() is a different beast. Attached is a quick hack which makes it work, obviously not the right way, but I'm going to bed now.

Ok, this is more elegant, should be green.

Note, like xjm says in #48, the image effects are the only weird stuff left, but I'm going to bed now :)

Discussed with @swentel. We've three things for followups from this:

1. Decide how to respond when someone does add a UUID.
2. Sort what to do with the field staging tests. From @swentel:
   

   well, I think we'll probably need to refactor those field tests to copy those field files to staging through entity api so they get a uuid
   in the current field config test, I'm just directly copying those files with file_unmanaged_copy() - probably not a clean way todo it

3. Sort out what to do with the image effects. From @swentel:
   

   as for those imageeffects, we'll probably have to look at ImageEffectBag::updateConfiguration() which assigns a uuid - I think we can fix that in a follow up

1. I'm in favour of throwing an exception - if someone does not test enabling a module then so be it - it will not take long to fix.
2. It's a test - does it matter if UUIDs are hard coded?

3. +++ b/core/modules/image/config/image.style.large.yml
   @@ -9,5 +8,4 @@ effects:
   -    uuid: ddd73aa7-4bd6-4c85-b600-bdf2b1628d1d
   
   +++ b/core/modules/image/config/image.style.medium.yml
   @@ -9,5 +8,4 @@ effects:
   -    uuid: bddf0d06-42f9-4c75-a700-a33cafa25ea0
   
   +++ b/core/modules/image/config/image.style.thumbnail.yml
   @@ -9,5 +8,4 @@ effects:
   -    uuid: 1cfec298-8620-4749-b100-ccb6c4500779
   

   We should leave these uuids alone. Image effects in an image style are keyed by UUID so that a style can implement the same effect more than once. We should open a follow up to remove the WTF of the effect being keyed by the uuid and having it as a value.

Not even sure why effects in an image style have individual uuids to begin with.
Image styles were the first entities converted to cmi, even before thre were "config entities", they're almost as old as D7 code now...

Tour and Editor didn't declare the public uuid property, so no uuid was created on install. Alex suggested adding it to ConfigEntityBase. Maybe #2198377: Enforce UUID key name in configuration entities can clean this up further. Also brought back the uuid's in the image styles, that needs more discussion in a follow up.

Please note that the UUID keys for effects in an image style configuration are the result of a painfully hard fight to get right:

#1508872: Image effects duplicate on edit introduced them to fix a critical regression

And also:

#1782244: Convert image styles $style array into ImageStyle (extends ConfigEntity)
#1821854: Convert image effects into plugins
#1609418: Configuration objects are not unique

Unlike filters in a filter format configuration (in which each filter plugin can only exist once), an image style can contain multiple instances of the same effect. But yet, all effect instances still have to be properly identified (and diffed) when configuration is staged/synced.

But yeah, let's move that discussion into a separate/dedicated issue.

So lets get the followup mentioned in #47 created and move on with this.

Ok, awesome. This has always bugged me since it first went in, happy to see it removed. :)

On IRC I asked about why we don't have existing test coverage that needs to be updated as a result of this change. Basically, #2108813: Add fancier config import/export test scenario is still in needs review state, and we changed our minds regarding supporting the "export config from arbitrary site A and import into arbitrary site B" a few months back, so the original use case that this code was meant to support is no longer valid. $uuid moves the ConfigEntityBase because it'll still be saved in active config, just not in default.

Committed and pushed to 8.x. Thanks!

We'll need a run-through the existing change notices https://drupal.org/list-changes/published?keywords_description=uuid&to_b... to check for references to this property. In particular, https://drupal.org/node/2153709 needs to be deleted.

I cleaned up examples of UUIDs in default config in all the change records, and unpublished https://drupal.org/node/2153709 and marked it reverted.

Added followup #2201501: Throw an exception if a UUID is defined in default configuration. We still need one for the image effect... stuff...