It is possible to validate configuration using schema prior to importing. This will make the configuration import more robust by doing a pre flight check that what is being provided is sane.

CommentFileSizeAuthor
#38 reroll_diff_27-38.txt7.65 KBravi.shankar
#38 2414951-38.patch10.19 KBravi.shankar
#27 2414951-27.patch10.22 KBeffulgentsia
#21 2414951-21.patch10.07 KBsavkaviktor16@gmail.com
#6 2414951.6.patch11.66 KBalexpott
#1 2414951.1.patch6.97 KBalexpott

Issue fork drupal-2414951

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Parent issue: » #1842956: [Meta] Implement event listeners to validate imports
Related issues: +#2414953: Element uses \Drupal::service() as a service locator and prevents injecting a custom typed config manager in config
StatusFileSize
new 2414951.1.patch6.97 KB

Let's see what breaks. I've created #2414953: Element uses \Drupal::service() as a service locator and prevents injecting a custom typed config manager in config to handle something we need to do even if we decide not to do this.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Also to make this performant we really need to get #2411689: Use a MemoryBackend in StorageComparer so that configuration import validators don't have to reread data from disk or the db done. Since without that we'll be reading all the configuration many many times.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work

I think this is a great idea.

  1. +++ b/core/lib/Drupal/Core/Config/TypedConfigManager.php
@@ -295,4 +296,21 @@ public function hasConfigSchema($name) {
+  protected function alterDefinitions(&$definitions) {
+    $existing = array_keys($definitions);
+    parent::alterDefinitions($definitions); // TODO: Change the autogenerated stub
+    if ($existing !== array_keys($definitions)) {
+      // Throw an exception.
+    }
+  }

+++ b/core/lib/Drupal/Core/Plugin/DefaultPluginManager.php
@@ -224,9 +224,7 @@ protected function findDefinitions() {
-    if ($this->alterHook) {
-      $this->moduleHandler->alter($this->alterHook, $definitions);
-    }
+    $this->alterDefinitions($definitions);

@@ -242,4 +240,16 @@ protected function findDefinitions() {
+  /**
+   * Invokes the alter hook if set.
+   *
+   * @param $definitions
+   *   The array of definitions found during discovery.
+   */
+  protected function alterDefinitions(&$definitions) {
+    if ($this->alterHook) {
+      $this->moduleHandler->alter($this->alterHook, $definitions);
+    }
+  }
+

    This is great, but would be a different issue, no? 

  2. +++ b/core/lib/Drupal/Core/EventSubscriber/ConfigImportSubscriber.php
@@ -35,6 +48,26 @@ public function onConfigImporterValidate(ConfigImporterEvent $event) {
+          if ($errors === FALSE) {
+            $event->getConfigImporter()
+                  ->logError($this->t('No schema for !config_name', array('!config_name' => $name)));
+          }

    If we want to keep the schema optional, we should not make this condition an error.

The last submitted patch, 1: 2414951.1.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review
Issue tags: +Needs tests
StatusFileSize
new 2414951.6.patch11.66 KB

Rebased the patch on the latest in #2414953: Element uses \Drupal::service() as a service locator and prevents injecting a custom typed config manager in config and refactored the event to make it as easy as possible to read. Also surprise surprise we found some invalid config lying about :).

Re #4.1 yep - going to create a new issue.
Re #4.2 yep - fixed.

We need to add some tests of schema errors being caught. The ConfigImportAll test is proof that schema validation for uninstalled modules is working nicely.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

I think its key to have a module installed in that test.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

So what exactly happens if an error is logged? Does it still import or is it aborted?

Enforcing that only valid configuration can be imported seems like a huge change to HEAD, there are still modules that don't have valid configuration schema.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

I talked about this with @Berdir in IRC - he's in favour of moving this to contrib since partial or wrong schemas are common in contrib. One problem with this is that how will they ever get fixed?

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

I hope translators will put some pressure on them (oh, did I write that?) I mean they will help fix things.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

One option would be some kind of warnings, that aren't blocking the import. (a collapsed fieldset that lists the first few, or something).

We have tests that validate it by default, and I think the test coverage for frequently used contrib modules will be much better in 8.x than it was in earlier versions.

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

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should 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.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should 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.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should 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.

anavarre’s picture

anavarre
Primary language French
Location 🇪🇺
commented
Version: 8.3.x-dev » 8.4.x-dev

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.

Status: Needs review » Needs work

The last submitted patch, 6: 2414951.6.patch, failed testing. View results

james.williams’s picture

james.williams
Location Coventry, UK
commented
Issue tags: +Needs reroll
savkaviktor16@gmail.com’s picture

savkaviktor16@gmail.com commented
Status: Needs work » Closed (outdated)
Issue tags: -Needs reroll
StatusFileSize
new 2414951-21.patch10.07 KB

Re-rolled

savkaviktor16@gmail.com’s picture

savkaviktor16@gmail.com commented
Status: Closed (outdated) » Needs review

Status: Needs review » Needs work

The last submitted patch, 21: 2414951-21.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

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.

tr’s picture

tr commented
Related issues: +#2150803: Config::$name not validated properly
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +CMI 2.0 candidate, +API-First Initiative
Related issues: +#2971727: Validate configuration upon saving (prevents invalid config imports a.o.), +#2969065: Use typed config validation constraints for validation of cdn.settings simple config, +#1928868: Typed config incorrectly implements Typed Data interfaces, +#2952037: [meta] Add constraints to all simple configuration, +#2300677: JSON:API POST/PATCH support for fully validatable config entities

Marked #2971727: Validate configuration upon saving (prevents invalid config imports a.o.) as a duplicate of this.

effulgentsia’s picture

effulgentsia commented
Status: Needs work » Needs review
StatusFileSize
new 2414951-27.patch10.22 KB

Reroll

Status: Needs review » Needs work

The last submitted patch, 27: 2414951-27.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

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.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#2164373: [META] Untie config validation from form validation — enables validatable Recipes, decoupled admin UIs …

As of #2164373-34: [META] Untie config validation from form validation — enables validatable Recipes, decoupled admin UIs …, this is starting to become more important 😊

ravi.shankar’s picture

ravi.shankar commented

Working on it.

ravi.shankar’s picture

ravi.shankar commented
StatusFileSize
new 2414951-38.patch10.19 KB
new reroll_diff_27-38.txt7.65 KB

Added reroll of patch #27 on Drupal 10.1.x.

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.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +validation
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Configuration schema

This is now actually becoming feasible and hence relevant — see #3324984-33: Create test that reports % of config entity types (and config schema types) that is validatable 😊

narendraR made their first commit to this issue’s fork.

narendraR opened merge request !8519

bbrala made their first commit to this issue’s fork.

bbrala’s picture

bbrala
Location Netherlands
commented

Rebased and brought codestyle (and phpstan) up to par.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.