Enforce writing of config changes during import, even if the module callback handled it already

Spin-off from #1609760-62: hook_image_style_*() is not invoked for image styles upon Image module installation

Problem

• Module APIs being invoked during a config import operation may not save the to be imported configuration (at all), and as such, cannot be trusted.

Goal

• Make the import mechanism save every Config object during an import, regardless of whether is has been handled by the owning module callback already.

Proposed solution

1. Invoke the MODULE_config_import_*() callback for a Config object to allow the owning module to take over the saving/deleting.
2. Overwrite the Config object saved by the module API with the exact data from the source storage (or respectively, enforce its deletion).

Details

• - Unknown -

Notes

(Discussion between @beejeebus and @sun)

• There has been an original assumption that when we import new config, we write out the values from the import tree exactly as they are.
• But now that we are actively involving the module API, it appears wrong to some of us to overrule the module API (which owns the config object).
• The provided examples about setting defaults and stuff do make a point. They are not necessarily correct, but they are cases we need to consider.
• Taking this "feature" into consideration, one could (hypothetically) write a new view config file with just, say, it's "name" property, and assume that Views will fill in a bazillion default fields for me. And there's not really something wrong with that.
• The original assumption was that such a config would be rejected by the import mechanism. Specifically with regard to scenarios, in which you push from dev/test to production. In that scenario, you export config and push them. And you do not expect to see differences.
• Technically, that is already the case. The module API just saves what it gets/imports (i.e., idempotence in ->load()->save()). So you do not see a difference.
• However, if we'd just simply overwrite what the module API wrote before, then we apparently would exactly not show you a difference.
• If you'd want to see/catch any errors/differences during/after import, then we'd rather have to invoke the module API, and after doing so, reload what has been written, and compare that with the config that was supposed to be written originally.
• So, on one side, we have this idea of "what goes in from an import is what goes out" vs "modules may augment imported config instead of rejecting it". The idea of setting defaults etc seems to be very useful. That would be a real feature, but it does complicate the assumptions around the system.
• The idea of strictly saving what's supposed to be imported is well understood; many of us assumed that for a long time. But it does not necessarily make sense to enforce this rule at all times (specifically with regard to installing a module's default config).
• It might be possible to add a flag to the import mechanism to conditionally enable that post-module-import-callback-validation for "true" imports from exported config files -- although it's not clear whether that would help.
• You do not really care for whether the default config ends up 1:1 in the system after installation. You only care about that in the situation in which you import from exported files (or more generally, the passive store). In that case, we can make all sorts of assumptions about the target system.
• So we could add another argument to config_import_invoke_owner(), which has a reasonable default, but which essentially enables that post-import-validation after invoking the module API by comparing the effective result with the expected result, and somehow, erroring out on a mismatch. That, in order to validate "what gets written is the same as what goes in."
• Actually, the module callback might have to decide on its own whether the config to be imported shows essential differences, so as to make a clear difference between error conditions and minor diversions.
• In any case, the module code has no idea about the system it is being installed on. It could be a fresh install, could d.o., whatever. So, when importing module config at install time, it makes sense that what gets written may differ, because many modules augment others, or add something to each view or whatever. So you can't look at the set of module default config files and say "ok, that is what is in the active store".
• However, this is also the reality of having to deal with a multi-dimensional module scenario. Because you do not know what the target system/environment is.
• In turn, the question pretty much boils down to: In a staging scenario, you can and should reasonably expect identical import results if your target system/environment is 100% identical to the previous dev/stage environment. And, there is no reason why the import would yield different results if everything is identical.
• You can only get differences if the target environment is not the same. And this is where the conflict with original assumptions appears.
• The original assumption is that this does not feel right. The system should do what it is supposed to do, the obvious thing. In natural language, "import this config", just that.
• However, if you want to make the system to do exactly what you think it should do, then you just let it do its thing. If, for whatever reason, that turns out to not yield the result you were intending, then you better know by seeing differences, instead of failing + overwriting silently. That's a valuable point.
• At the point you're writing the exact data that lives in files, you're eliminating the potential difference. So we should probably blow up upon detecting a difference. Although that begs the question of "how".
• There are two options: 1) Just overwrite what the module API might have written. 2) Detect a diff and blow up.
• But it really boils down to trusting the module API, or not. If we do not trust it, then we must always write, and we don't care if what is written doesn't match what was exported. If we trust it, then we trust what it writes.
• The original assumption also contained: "Look, I exported that tree, that's what prod looks like." + We're building something to move config between environments. But now, we're building something that allows you to take some config from anywhere, try to import it.
• If you want to see what prod looks like, just export it. That's always going to be 1:1 export. But if you want import that tree of config, then you need a system/environment that is 100% identical to the one it was exported. If that is the case, no differences.
• Only if the target system/environment is NOT identical, you naturally get differences.
• So essentially, an exported tree from a dev/staging environment is just a "guide" to what will be written to your prod system.
• And yes, we're going to run into epic/monster bugs with the export/import mechanism, since we didn't even think a minute about module update paths and version conflicts. But luckily, that's a topic for code freeze.
• It probably wouldn't be just a guide, if there was an overall system compatibility validation operation happening before the import -- e.g., comparing enabled modules, their versions, Drupal version, PHP version, PHP extensions, etc.pp. - whereas that overall system validation would be an attempt to validate 80-99% system equality before the import.
• And that's pretty much the essence: If you make the assumption that the source and target systems are 100% identical, then no module API will yield any differences in what is being saved. It is probably safe to make that assumption.
• But in light of that, an overall system validation as a first step before trying to import exported config might make sense, too. That overall system validation might help us to resolve conflict scenarios later -- e.g., if you push new code + new config at the same time, the system would be able to tell you that you first need to run update.php, before you can execute the import.
• As a result, the only disagreement is whether to trust module APIs to write changes or not. It appears to be a minor detail, but apparently questions the entire concept.

Counter arguments

• Overwriting the config object after the module API saved it will potentially overwrite differences. The effective result of what is being saved after being passed through a module API can depend on other factors in the system/environment. Doing so will only hide errors and effective differences.

  For example, a module API may reasonably decide to inject its current API version into each configurable thingie's object that is saved through its API. Other reasonable examples would be:

  • Ensuring default values for all properties.
  • Injecting system/environment-specific values. (e.g., auto-generating the system.cron:key in case none is defined yet)
  • Validating references to other resources in the system, and possibly adjusting a status/enabled flag depending on their existence in the target system.
  • ...
• Making the overwrite less rigid by writing the data in $new_config does not account for the fact that there is no hard requirement for the module API to use $new_config as-is/literally. In other words:
  1. There is no guarantee that the module API saves the $new_config object. This breaks the assumption:
     

       $config = config($new_config->get());
       $config->save();
     

  2. There is no guarantee that the module API uses the $new_config object. There is no requirement for the module API to be based on the literal Config object as is. A module may use a completely different way for managing its dynamic configuration thingies. Have a look at Image module, whose image styles happen to be plain arrays:
     

       $style = $new_config->get();
       return image_style_save($style);
     

  This essentially repeats #132 and #136 from #1447686-132: Allow importing and synchronizing configuration when files are updated