Problem/Motivation
Quoting #2969065: Use typed config validation constraints for validation of cdn.settings simple config:
Since #1928868: Typed config incorrectly implements Typed Data interfaces, it's possible to validate configuration, even simple configuration, through Symfony validation constraints. Just like we validate entities. #2952037: [meta] Add constraints to all simple configuration exists to add it to all of core's simple configuration.
At the recent DrupalCon Nashville, we discussed how Drupal's API-First Initiative can make config entities validatable and hence modifiable (see #2300677-241: JSON:API POST/PATCH support for fully validatable config entities and #2300677-242: JSON:API POST/PATCH support for fully validatable config entities). That got me thinking.
Why not add validation constraints to
cdn.settings? That'd allow both the CDN UI module to use it and config imports to also use it.
(Emphasis added.)
When importing configuration, no validation occurs. Consequence: it's fairly easy to break a site in obvious or subtle ways (depending on which configuration).
Proposed resolution
-
The patch at #2969065: Use typed config validation constraints for validation of cdn.settings simple config is explicitly adding a
ConfigEvents::SAVEevent subscriber that invokes the configuration system's validation capabilities. Specifically for the CDN module's simple config. This works fine:
Why not move this into core? That should be the end goal.
Of course, it's not realistic to enable this right away. But any configuration that is fully covered by validation constraints (see #2300677-241: JSON:API POST/PATCH support for fully validatable config entities and #2300677-242: JSON:API POST/PATCH support for fully validatable config entities) could (and I'd argue should) have this happen automatically.
-
Sadly, the above only happens after saving (because the data is written first by
\Drupal\Core\Config\Config::save()and thenConfigEvents::SAVEis dispatched). But that's still a step forward compared to today, where we'll just silently accept the data. It seems the real solution is for config importing to first validate all config it will import, and only then save it. Perhaps that should be done as part of this issue, perhaps it's a follow-up, perhaps it's a blocker. TBD.
Remaining tasks
TBd
User interface changes
TBD
API changes
TBD
Data model changes
None.
Comments
Comment #2
dawehnerI think there are two points where we need to ensure that we fire up validation:
\Drupal\Core\Config\ConfigImporter::validatedirectly. I think running$typed_config->validate()and stop would be certainly nice$simple_config->validate()which has a similar logic asContentEntityBase, as in, from outside you can ensure that these config objects are validated before saving. I fear this is needed in order to not break BC for code which manually saves config with maybe not 100% valid data :(Comment #4
hchonovI just had the same idea and voila there is already an issue for it :).
I think that there is one more point :). It would be really nice if we validate the configs also when installing profile or modules. For that we would have to validate the config to be saved in
\Drupal\Core\Config\ConfigInstaller::createConfiguration()Comment #6
wim leersAlex Pott pointed me to #2414951: Validate configuration schema before importing configuration. Marking this as a duplicate.