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

  1. The patch at #2969065: Use typed config validation constraints for validation of cdn.settings simple config is explicitly adding a ConfigEvents::SAVE event 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.

  2. Sadly, the above only happens after saving (because the data is written first by \Drupal\Core\Config\Config::save() and then ConfigEvents::SAVE is 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

Wim Leers created an issue. See original summary.

dawehner’s picture

I think there are two points where we need to ensure that we fire up validation:

  • When we import the actual configuration. One could do that in \Drupal\Core\Config\ConfigImporter::validate directly. I think running $typed_config->validate() and stop would be certainly nice
  • I think we should introduce $simple_config->validate() which has a similar logic as ContentEntityBase, 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 :(

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.

hchonov’s picture

I just had the same idea and voila there is already an issue for it :).

I think there are two points where we need to ensure that we fire up validation:

  • When we import the actual configuration. One could do that in \Drupal\Core\Config\ConfigImporter::validate directly. I think running $typed_config->validate() and stop would be certainly nice
    I
  • think we should introduce $simple_config->validate() which has a similar logic as ContentEntityBase, 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 :(

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()

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.

wim leers’s picture

Status: Active » Closed (duplicate)
Related issues: +#2414951: Validate configuration schema before importing configuration

Alex Pott pointed me to #2414951: Validate configuration schema before importing configuration. Marking this as a duplicate.