Problem

  • Default configuration files of modules may contain UUIDs from the originating site on which the config was created.
  • These UUIDs of newly created configuration objects are not universally unique, but identical on all sites instead.

Proposed solution

  1. Force-create a new UUID for each configuration entity object that is imported from default configuration during initial installation of an extension.

Notes

  • Given this and other existing diverging behavior of the default config import process:
    1. The original assumption that the default config import vs. the active/staging store synchronization would be the same is no longer really true.
    2. Next to custom business logic like force-creating new UUIDs, the default config also uses a single, non-swappable file format.
    3. It's probably time to decouple of the default config import code from the config syncing code. Which probably allows us to kill the new isSyncing() flag, too.
CommentFileSizeAuthor
#6 drupal8.config-default-uuid.6.patch953 bytessun
#2 drupal8.config-default-uuid.2.patch734 bytessun
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

"Default configuration files of modules may contain UUIDs from the originating site on which the config was created."

i don't think that's true, and should be treated as a bug.

"It's probably time to decouple of the default config import code from the config syncing code. Which probably allows us to kill the new isSyncing() flag, too."

what does this look like?

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Active » Needs review
FileSize
drupal8.config-default-uuid.2.patch734 bytes

Perhaps we can do it as simple as this?

@beejeebus:

Interpreting it as a "bug" is probably correct. But at the same time, it is also poor DX, because we require all developers to manually edit and manipulate the default configuration files of their installation profiles, modules, and themes.

Enforcing this is a no-brainer for the system and just a single line of code. That's a very cheap win to (1) guarantee universally unique UUIDs under all possible circumstances and (2) vastly improve DX.

The entire default config import process still appears to be located in the single procedural config_install_default_config() function.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs review » Needs work

alexpott, yched and i were talking about this in IRC today, and generally came to the conclusion that blanking the UUID seems fine. i'm kinda meh about it, so i guess we can be more forgiving rather than throw an exception.

this patch needs to take in to account the fact that entities can specify a UUID key, so we can't just look for 'uuid' :-\

other than that, all this patch needs is a test and i'd happily RTBC it.

yched’s picture

yched CreditAttribution: yched commented

As discussed in IRC, not sure what's best:
- enforcing manual cleanup of yml shipped in default config to remove UUIDs is a DX burden
- allowing leftover UUIDs and and silently ignoring them is misleading - "Yeah, there's a UUID in there, but don't trust it, it's going to be discarded. Yeah, you can trust the rest".

Fully agreed on clearly separating the "default config import" & "config sync" code paths, Those are really different processes with very different logic and requirements and nasty cases. In all the recent mind bending discussions, I've tried hard to talk about them as distinct processes with distinct names :)

I don't see how isSyncing() can go away, though. It's used in the sync process so that code like MyConfigEntityType::postSave() or hook_entity_xxx_create() can avoid trigering race conditions. See #2069373: Race conditions on import if CUD on ConfigEntity A triggers changes in ConfigEntity B. Not related to default config objects.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

@beejeebus:

entities can specify a UUID key, so we can't just look for 'uuid'

The Configuration Entity system (cf. ConfigEntityBase/ConfigStorageController) hard-codes a UUID property for all configuration entities.

In preparing this patch, I double-checked the existing code, and it appears that we're not consistently relying on $entity->{$this->uuidKey} from the entity info.

I'm actually not sure whether it makes sense to continue to make the uuid property customizable, instead of hard-coding a uuid property name for all configuration entities.

Though that might be OT here and a different discussion, so if necessary, I'm happy to adjust the patch to look up the uuidKey from the entity info instead.

@yched:

allowing leftover UUIDs and and silently ignoring them is misleading

That's a valid concern. However, I guess the best course of action to prevent that would be a new scaffolding facility to "Export this config entity here into default config for my module", which would export the config entity and remove the UUID before writing — would obviously work on writable filesystems only, but perhaps a start to improve DX and prevent the situation from happening.

In any case, I think it makes sense to enforce this behavior within the system, so that there is an actual guarantee for the expected behavior. Especially if it's as simple as this.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs work » Needs review
FileSize
drupal8.config-default-uuid.6.patch953 bytes

Respecting a custom uuidKey, I assume the code would have to look like this.

The last submitted patch, 2: drupal8.config-default-uuid.2.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 6: drupal8.config-default-uuid.6.patch, failed testing.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

re the uuidKey, i'd be ok with killing that feature also. i'd support doing that in this patch, but i'd want that to be ok with yched before we do it.

yched’s picture

yched CreditAttribution: yched commented

As a personal opinion, I'd approve killing the uuidKey flexibility too...

But that looks like a far reaching change (across all config + content entity types ?), for which @fago & @Berdir would be the ones to ask for buy-in, more than myself.

In short, big derailing potential IMO :-)

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Not sure about uuidKey, what we do need to make sure is that it continues to be optional for non-configuration entities, e.g. aggregator feed items only have an externally-provided guid key and not a uuid.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Parent issue: #2121751: [META] Making configuration synchronisation work »
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Issue tags: +Needs tests

We should either proceed with this or add a test to confirm that UUIDs are maintained during a config install

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented

#2201501: Throw an exception if a UUID is defined in default configuration was closed as a duplicate of this issue.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

I think we should proceed with this issue but only enforce an empty UUID when installing modules or themes. Profiles should get a pass because maybe its a distribution like OpenAtrium and they want to be have a direct link between distribution provided config and the active configuration.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Version: 8.0.x-dev » 8.1.x-dev
Status: Needs work » Postponed

Discussed with alexpott and postponed to 8.1.x. This problem is not as severe now as it was when the original issue was filed, thankfully.

https://www.drupal.org/core/d8-allowed-changes

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Issue tags: +Triaged core major
jweowu’s picture

jweowu CreditAttribution: jweowu commented
Related issues: +#2179537: Drupal 8 has a broken UUID generator

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.