If an update hook modifies configuration, then old configuration is imported, the changes made by the update hook are forever lost.

Since we would presumably fix this in D8 too, I'm filing it against 8.8.x (which is the current bugfix support branch). Patches can be tested against other branches as needed when they're uploaded without changing the issue's version selector.

The issue will be automatically updated to 8.9.x after the last 8.8.x bugfix release. Thanks!

As a developer, it would be interesting that updb do the following:

1. check the current sync state of the config against the yaml config files: I can evaluate with a diff list if the updb may be run safely (aka I did not forget to import important configs before applying the updb)
2. prompt if ok to start the update process
3. do the updb as usual: show the updates that will be run + prompt
4. warn me the current sync state of the config again: if configs are changed, it tells me that configs need to be exported to finish the update process properly (additionnally, ask me to export rightaway).

On top of this, a --deployment or --no-config-check flag could be added for scenarios when checking the configs are unnecessary (or maybe reuse the -y flag?).

The only unsupported scenario is when the update is triggered in the UI (update.php). A similar functionality has to be implemented in the update.php: UI to show the config diff, ask to run the update, show the diff again to warn the config changes.

Could this be enough to help with avoiding CMI mistakes? This has the advantage that the update functionality itself is not modified, it only asks for confirmation and warns.

I am adding two related issues:
#2762235: Ensure that ConfigImport is taking place without outstanding updates This will effectively enforce the order of operations (ie you can not cim before updb)
#3046903: [discussion] Environment specific modules with updates. This seems unrelated at first but will maybe be needing a similar approach than what #5 is suggesting.

Worth noting is also the drush deploy command.

IMO this is not a supported workflow. You *must* get your configuration right before you run a config import. Specifically, the config in the import must represent the values AFTER hook_updated_n runs. The developer workflow I advocate is to run the hook_update_n() locally, run drush config:export, and and then commit the results. Only then is the saved config ready to be imported.

I'm sure docs could be improved in this area.

@moshe weitzman, what about the fields? If the fields storage and field instance was created in update/post_update hook and then exported locally to config/sync with some uuid, then on remote server the uuid might be different and then the field will be re-created on configuration import. If there is any post_update that migrates some data to those fields, that data will be lost, because config import will delete the field and create it again.

If there is any post_update that migrates some data to those fields, that data will be lost, because config import will delete the field and create it again.

Content should be entered via a HOOK_deploy_NAME(). Thats a new hook introduced by the drush deploy command. See https://www.drush.org/deploycommand/

Also, you don't need hook_update_n() to propagate a field creation. Do it locally in the GUI, and then config:export and the commit the result. No more is needed. Or just use base fields which are all in code.

Thanks, but that deploy hook is only available with Drush 10, or? and the update hook I use to create commerce_subscription field, there is no UI for that.

Correct on Drush 10. Ideally core would have that hook but it doesnt yet acknowledge that drupal deployment is a thing.

This issue recently affected us with the release of Drupal 10.1. #1845004: Replace custom password hashing library with PHP password_hash() included an update hook to enable the new phpass core module. Once that update hook ran, the module was enabled only to be disabled when config:import was subsequently run.

We have automated tests that check the status of Drupal config state, but these did not fail because once config:import had run (via drush deploy), the configuration on disk matched the active configuration as far as Drupal was concerned.

If we are allowing folks to believe that configuration on disk is their source of truth and then modifying active configuration with an update hook, this seems like it will inevitably cause issues like this.

It seems to me like there's a fundamental contradiction to running database updates and configuration updates all in a single action.

It's already very complicated to wrestle with the concept of "correct configuration" when you have site administrators making configuration updates and developers making configuration updates in parallel.

What if configuration updates in code were given special treatment? We could have a separate one-time operation from updb [hook_install_configuration_N(): returns an array of configuration files to be re-imported from a module's /config/*/ directories; is triggered by 'drush update-configuration' or visiting /update-config.php]

Then, the "best practice" could be:
1. updb
2. cim
3. upcfg

(and then: export that config into code!)

If we tracked the executed upcfg functions in their own table with a date or "invocation" column, we could provide a way to "re-run" an invocation if a site accidentally ran CIM after upcfg.

At Lullabot, we wrote up an Architecture Decision Record (ADR) to deal with a similar situation.

https://architecture.lullabot.com/adr/20210924-drupal-build-steps/

Depends on developers using hook_post_update_NAME() . [ 1845004 did use a hook_post_update_NAME: system_post_update_enable_password_compatibility ]

Oh, and that ADR works in conjunction with this one: https://architecture.lullabot.com/adr/20211212-config-status-check/

Does that help?

A PR is in progress for drush deploy to catch this problem and stop the deployment - https://github.com/drush-ops/drush/pull/5713