Add legacy PLATFORM_SCHEMA config validation (#21072)

* Add legacy PLATFORM_SCHEMA config validation

* Fix tests

* Lint

* Add persistent notification

* Update bootstrap.py

homeassistant/bootstrap.py

@@ -192,6 +192,23 @@ async def async_from_config_dict(config: Dict[str, Any],
             '\n\n'.join(msg), "Config Warning", "config_warning"
         )
 
+    # TEMP: warn users of invalid extra keys
+    # Remove after 0.92
+    if cv.INVALID_EXTRA_KEYS_FOUND:
+        msg = []
+        msg.append(
+            "Your configuration contains extra keys "
+            "that the platform does not support (but were silently "
+            "accepted before 0.88). Please find and remove the following."
+            "This will become a breaking change."
+        )
+        msg.append('\n'.join('- {}'.format(it)
+                             for it in cv.INVALID_EXTRA_KEYS_FOUND))
+
+        hass.components.persistent_notification.async_create(
+            '\n\n'.join(msg), "Config Warning", "config_warning"
+        )
+
     return hass
 
 

homeassistant/helpers/config_validation.py

@@ -34,6 +34,7 @@ OLD_ENTITY_ID_VALIDATION = r"^(\w+)\.(\w+)$"
 # persistent notification. Rare temporary exception to use a global.
 INVALID_SLUGS_FOUND = {}
 INVALID_ENTITY_IDS_FOUND = {}
+INVALID_EXTRA_KEYS_FOUND = []
 
 
 # Home Assistant types
@@ -633,8 +634,61 @@ def key_dependency(key, dependency):
 
 
 # Schemas
+class HASchema(vol.Schema):
+    """Schema class that allows us to mark PREVENT_EXTRA errors as warnings."""
 
-PLATFORM_SCHEMA = vol.Schema({
+    def __call__(self, data):
+        """Override __call__ to mark PREVENT_EXTRA as warning."""
+        try:
+            return super().__call__(data)
+        except vol.Invalid as orig_err:
+            if self.extra != vol.PREVENT_EXTRA:
+                raise
+
+            # orig_error is of type vol.MultipleInvalid (see super __call__)
+            assert isinstance(orig_err, vol.MultipleInvalid)
+            # pylint: disable=no-member
+            # If it fails with PREVENT_EXTRA, try with ALLOW_EXTRA
+            self.extra = vol.ALLOW_EXTRA
+            # In case it still fails the following will raise
+            try:
+                validated = super().__call__(data)
+            finally:
+                self.extra = vol.PREVENT_EXTRA
+
+            # This is a legacy config, print warning
+            extra_key_errs = [err for err in orig_err.errors
+                              if err.error_message == 'extra keys not allowed']
+            if extra_key_errs:
+                msg = "Your configuration contains extra keys " \
+                      "that the platform does not support.\n" \
+                      "Please remove "
+                submsg = ', '.join('[{}]'.format(err.path[-1]) for err in
+                                   extra_key_errs)
+                submsg += '. '
+                if hasattr(data, '__config_file__'):
+                    submsg += " (See {}, line {}). ".format(
+                        data.__config_file__, data.__line__)
+                msg += submsg
+                logging.getLogger(__name__).warning(msg)
+                INVALID_EXTRA_KEYS_FOUND.append(submsg)
+            else:
+                # This should not happen (all errors should be extra key
+                # errors). Let's raise the original error anyway.
+                raise orig_err
+
+            # Return legacy validated config
+            return validated
+
+    def extend(self, schema, required=None, extra=None):
+        """Extend this schema and convert it to HASchema if necessary."""
+        ret = super().extend(schema, required=required, extra=extra)
+        if extra is not None:
+            return ret
+        return HASchema(ret.schema, required=required, extra=self.extra)
+
+
+PLATFORM_SCHEMA = HASchema({
     vol.Required(CONF_PLATFORM): string,
     vol.Optional(CONF_ENTITY_NAMESPACE): string,
     vol.Optional(CONF_SCAN_INTERVAL): time_period

tests/test_setup.py

@@ -90,7 +90,7 @@ class TestSetup:
                 }
             })
 
-    def test_validate_platform_config(self):
+    def test_validate_platform_config(self, caplog):
         """Test validating platform configuration."""
         platform_schema = PLATFORM_SCHEMA.extend({
             'hello': str,
@@ -109,7 +109,7 @@ class TestSetup:
             MockPlatform('whatever',
                          platform_schema=platform_schema))
 
-        with assert_setup_component(0):
+        with assert_setup_component(1):
             assert setup.setup_component(self.hass, 'platform_conf', {
                 'platform_conf': {
                     'platform': 'whatever',
@@ -117,11 +117,13 @@ class TestSetup:
                     'invalid': 'extra',
                 }
             })
+            assert caplog.text.count('Your configuration contains '
+                                     'extra keys') == 1
 
         self.hass.data.pop(setup.DATA_SETUP)
         self.hass.config.components.remove('platform_conf')
 
-        with assert_setup_component(1):
+        with assert_setup_component(2):
             assert setup.setup_component(self.hass, 'platform_conf', {
                 'platform_conf': {
                     'platform': 'whatever',
@@ -132,6 +134,8 @@ class TestSetup:
                     'invalid': True
                 }
             })
+            assert caplog.text.count('Your configuration contains '
+                                     'extra keys') == 2
 
         self.hass.data.pop(setup.DATA_SETUP)
         self.hass.config.components.remove('platform_conf')
@@ -183,7 +187,7 @@ class TestSetup:
             assert 'platform_conf' in self.hass.config.components
             assert not config['platform_conf']  # empty
 
-    def test_validate_platform_config_2(self):
+    def test_validate_platform_config_2(self, caplog):
         """Test component PLATFORM_SCHEMA_BASE prio over PLATFORM_SCHEMA."""
         platform_schema = PLATFORM_SCHEMA.extend({
             'hello': str,
@@ -204,7 +208,7 @@ class TestSetup:
             MockPlatform('whatever',
                          platform_schema=platform_schema))
 
-        with assert_setup_component(0):
+        with assert_setup_component(1):
             assert setup.setup_component(self.hass, 'platform_conf', {
                 # fail: no extra keys allowed in platform schema
                 'platform_conf': {
@@ -213,6 +217,8 @@ class TestSetup:
                     'invalid': 'extra',
                 }
             })
+            assert caplog.text.count('Your configuration contains '
+                                     'extra keys') == 1
 
         self.hass.data.pop(setup.DATA_SETUP)
         self.hass.config.components.remove('platform_conf')
@@ -234,7 +240,7 @@ class TestSetup:
         self.hass.data.pop(setup.DATA_SETUP)
         self.hass.config.components.remove('platform_conf')
 
-    def test_validate_platform_config_3(self):
+    def test_validate_platform_config_3(self, caplog):
         """Test fallback to component PLATFORM_SCHEMA."""
         component_schema = PLATFORM_SCHEMA_BASE.extend({
             'hello': str,
@@ -255,15 +261,16 @@ class TestSetup:
             MockPlatform('whatever',
                          platform_schema=platform_schema))
 
-        with assert_setup_component(0):
+        with assert_setup_component(1):
             assert setup.setup_component(self.hass, 'platform_conf', {
                 'platform_conf': {
-                    # fail: no extra keys allowed
                     'platform': 'whatever',
                     'hello': 'world',
                     'invalid': 'extra',
                 }
             })
+            assert caplog.text.count('Your configuration contains '
+                                     'extra keys') == 1
 
         self.hass.data.pop(setup.DATA_SETUP)
         self.hass.config.components.remove('platform_conf')