Merge pull request #7776 from ckan/strict-config-by-default

Strict configuration parsing by default

changes/7677.removal

@@ -1 +1 @@
-`homepage_style` variable has been removed. `layout1.html` code has been moved into `home/index.html` as it will be the only layout available.
+ The ``ckan.homepage_style`` configuration options and the ``homepage_style`` variable have been removed. ``layout1.html`` code has been moved into ``home/index.html``, as it will be the only layout available.

changes/7776.feature

@@ -0,0 +1,4 @@
+The default behaviour starting from CKAN 2.11 is the old ``strict`` mode, where CKAN will not
+start unless **all** config options are valid according to the validators defined in the
+:ref:`configuration declaration <_declare-config-options>`. For every invalid config option,
+an error will be printed to the output stream.

ckan/common.py

@@ -136,8 +136,7 @@ def get(self, key: str, default: Any = SENTINEL) -> Any:
         """
         if default is SENTINEL:
             default = None
-            is_strict = super().get("config.mode") == "strict"
-            if is_strict and key not in config_declaration:
+            if len(config_declaration) and key not in config_declaration:
                 log.warning("Option %s is not declared", key)
 
         return super().get(key, default)

ckan/config/config_declaration.yaml

@@ -172,20 +172,15 @@ groups:
             Flask-based route names.
 
       - key: config.mode
-        default: default
+        default: strict
         example: strict
         description: |
-          Controls the behavior of application when invalid values detected in
-          the ``config`` object.
-
-          In the ``default`` mode any invalid value is left unprocessed (i.e.,
-          it remains a ``str``). In addition, every invalid option is reported using
-          a log record with a ``WARNING`` level.
 
-          In the ``strict`` mode, CKAN will not start unless **all** config
-          options are valid according to the validators defined in the
-          configuration declaration. For every invalid config option, an error will be
-          printed to the output stream.
+          .. warning:: This configuration option has no effect starting from CKAN 2.11. The
+            default behaviour going forward is the old ``strict`` mode, where CKAN will not
+            start unless **all** config options are valid according to the validators defined in the
+            configuration declaration. For every invalid config option, an error will be
+            printed to the output stream.
 
   - annotation: Development settings
     options:

ckan/config/declaration/__init__.py

@@ -70,6 +70,9 @@ def __contains__(self, key: Union[Key, str]):
     def __getitem__(self, key: Key) -> Option[Any]:
         return self._options[key]
 
+    def __len__(self) -> int:
+        return len(self._options)
+
     def get(self, key: Union[str, Key]) -> Optional[Option[Any]]:
         """Return the declaration of config option or `None`.
         """

ckan/config/environment.py

@@ -144,21 +144,12 @@ def update_config() -> None:
 
     _, errors = config_declaration.validate(config)
     if errors:
-        if config.get("config.mode") == "strict":
-            msg = "\n".join(
-                "{}: {}".format(key, "; ".join(issues))
-                for key, issues in errors.items()
-            )
-            msg = "Invalid configuration values provided:\n" + msg
-            raise CkanConfigurationException(msg)
-        else:
-            for key, issues in errors.items():
-                log.warning(
-                    "Invalid value for %s (%s): %s",
-                    key,
-                    config.get(key),
-                    "; ".join(issues)
-                )
+        msg = "\n".join(
+            "{}: {}".format(key, "; ".join(issues))
+            for key, issues in errors.items()
+        )
+        msg = "Invalid configuration values provided:\n" + msg
+        raise CkanConfigurationException(msg)
 
     root = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 

ckan/lib/app_globals.py

@@ -42,7 +42,6 @@
     'ckan.site_id': {},
     'ckan.recaptcha.publickey': {'name': 'recaptcha_publickey'},
     'ckan.template_title_delimiter': {'default': '-'},
-    'ckan.homepage_style': {'default': '1'},
 
     # split string
     'search.facets': {'default': 'organization groups tags res_format license_id',

ckan/tests/cli/test_config.py

@@ -17,7 +17,6 @@ def invoke(*args):
 
 
 @pytest.mark.usefixtures("with_extended_cli")
-@pytest.mark.ckan_config("config.mode", "strict")
 class TestDescribe(object):
     def test_basic_invocation(self, command):
         """Command prints nothing without arguments;"""
@@ -111,7 +110,6 @@ def test_formats(self, fmt, loader, command):
 
 
 @pytest.mark.usefixtures("with_extended_cli")
-@pytest.mark.ckan_config("config.mode", "strict")
 class TestDeclaration(object):
     def test_basic_invocation(self, command):
         result = command("declaration")
@@ -148,7 +146,6 @@ def test_explicit(self, command):
 
 
 @pytest.mark.usefixtures("with_extended_cli")
-@pytest.mark.ckan_config("config.mode", "strict")
 class TestSearch(object):
     def test_wrong_non_pattern(self, command):
         result = command("search", "ckan")
@@ -195,7 +192,6 @@ def test_extra_plugin_pattern(self, command):
 
 
 @pytest.mark.usefixtures("with_extended_cli")
-@pytest.mark.ckan_config("config.mode", "strict")
 class TestUndeclared(object):
     def test_no_undeclared_options_by_default(self, command):
         result = command("undeclared", "-idatapusher", "-idatastore")
@@ -233,13 +229,6 @@ def test_no_errors_by_default_in_safe_mofe(self, command):
         assert not result.exit_code, result.output
 
     @pytest.mark.ckan_config("ckan.devserver.port", "8-thousand")
-    def test_invalid_port(self, command):
-        result = command("validate")
-        assert not result.exit_code, result.output
-        assert "ckan.devserver.port" in result.stdout
-
-    @pytest.mark.ckan_config("config.mode", "strict")
-    @pytest.mark.ckan_config("ckan.devserver.port", "8-thousand")
-    def test_invalid_port_in_strict_mode_prevents_application_initialization(self, command):
+    def test_invalid_port_prevents_application_initialization(self, command):
         result = command("validate")
         assert result.exit_code, result.stdout

ckan/tests/config/test_environment.py

@@ -63,7 +63,7 @@ def test_update_config_env_vars(ckan_config):
 
 @pytest.mark.ckan_config("ckan.site_url", "")
 def test_missing_siteurl():
-    with pytest.raises(RuntimeError):
+    with pytest.raises(CkanConfigurationException):
         environment.update_config()
 
 

ckan/tests/config/test_middleware.py

@@ -7,6 +7,7 @@
 from ckan import plugins as p
 from ckan.common import config, _
 from ckan.lib.helpers import url_for
+from ckan.exceptions import CkanConfigurationException
 
 
 class BlueprintPlugin(p.SingletonPlugin):
@@ -87,7 +88,7 @@ def test_beaker_secret_is_used_by_default(app):
 @pytest.mark.ckan_config(u"SECRET_KEY", None)
 @pytest.mark.ckan_config(u"beaker.session.secret", None)
 def test_no_beaker_secret_crashes(make_app):
-    with pytest.raises(RuntimeError):
+    with pytest.raises(CkanConfigurationException):
         make_app()
 
 

doc/maintaining/configuration.rst

@@ -74,60 +74,12 @@ options in the CKAN config file are not declared (and might have no effect).
 Declaring config options
 ------------------------
 
-The :py:class:`~ckan.plugins.interfaces.IConfigDeclaration` interface is
-available to allow extensions to declare their own config options.
-
-New config options can only be declared inside the
-:py:meth:`~ckan.plugins.interfaces.IConfigDeclaration.declare_config_options` method. This
-method accepts two arguments: a :py:class:`~ckan.config.declaration.Declaration`
-object that contains all the declarations, and a :py:class:`~ckan.config.declaration.Key`
-helper, which allows to declare more unusual config options.
-
-A very basic config option may be declared in this way::
-
-  declaration.declare("ckanext.my_ext.option")
-
-which just means that extension ``my_ext`` makes use of a config option named
-``ckanext.my_ext.option``. If we want to define the *default value* for this option
-we can write::
-
-  declaration.declare("ckanext.my_ext.option", True)
-
-The second parameter to
-:py:meth:`~ckan.config.declaration.Declaration.declare` specifies the default
-value of the declared option if it is not provided in the configuration file.
-If a default value is not specified, it's implicitly set to ``None``.
-
-You can assign validators to a declared config option::
-
-  option = declaration.declare("ckanext.my_ext.option", True)
-  option.set_validators("not_missing boolean_validator")
-
-``set_validators`` accepts a string with the names of validators that must be applied to the config option.
-These validators need to registered in CKAN core or in your own extension using
-the :py:class:`~ckan.plugins.interfaces.IValidators` interface.
-
-.. note:: Declared default values are also passed to validators. In addition,
-          different validators can be applied to the same option multiple
-          times. This means that validators must be idempotent and that the
-          default value itself must be valid for the given set of validators.
-
-If you need to declare a lot of options, you can declare all of them at once loading a dict::
-
-  declaration.load_dict(DICT_WITH_DECLARATIONS)
+.. note:: Starting from CKAN 2.11, CKAN will log a warning every time a non-declared
+  configuration option is accessed. To prevent this, declare the configuration options
+  offered by your extension using the methods below
 
-This allows to keep the configuration declaration in a separate file to make it easier to maintain if
-your plugin supports several config options.
-
-.. note:: ``declaration.load_dict()`` takes only python dictionary as
-          argument. If you store the declaration in an external file like a
-          JSON, YAML file, you have to parse it into a Python dictionary
-          yourself or use corresponding
-          :py:attr:`~ckan.plugins.toolkit.ckan.plugins.toolkit.blanket`. Read
-          the following section for additional information.
-
-Use a blanket implementation of the config declaration
-------------------------------------------------------
+Using a text file (JSON, YAML or TOML)
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 The recommended way of declaring config options is using the
 ``config_declarations``
@@ -137,9 +89,7 @@ package is installed inside your virtual environment). That is how CKAN
 declares config options for all its built-in plugins, like ``datastore`` or
 ``datatables_view``.
 
-Instead of implementing the
-:py:class:`~ckan.plugins.interfaces.IConfigDeclaration` interface, decorate the
-plugin with the ``config_declarations`` blanket::
+To use it, decorate the plugin with the ``config_declarations`` blanket::
 
   import ckan.plugins as p
   import ckan.plugins.toolkit as tk
@@ -273,6 +223,63 @@ only for explanation and you don't need them in the real file::
           # `legacy_key` is used, printing a deprecation warning in the logs.
           legacy_key: my_ext.legacy.flag.do_something
 
+The ``IConfigDeclaration`` interface
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+
+The :py:class:`~ckan.plugins.interfaces.IConfigDeclaration` interface is
+available to plugins that want more control on how their own config options are declared.
+
+New config options can only be declared inside the
+:py:meth:`~ckan.plugins.interfaces.IConfigDeclaration.declare_config_options` method. This
+method accepts two arguments: a :py:class:`~ckan.config.declaration.Declaration`
+object that contains all the declarations, and a :py:class:`~ckan.config.declaration.Key`
+helper, which allows to declare more unusual config options.
+
+A very basic config option may be declared in this way::
+
+  declaration.declare("ckanext.my_ext.option")
+
+which just means that extension ``my_ext`` makes use of a config option named
+``ckanext.my_ext.option``. If we want to define the *default value* for this option
+we can write::
+
+  declaration.declare("ckanext.my_ext.option", True)
+
+The second parameter to
+:py:meth:`~ckan.config.declaration.Declaration.declare` specifies the default
+value of the declared option if it is not provided in the configuration file.
+If a default value is not specified, it's implicitly set to ``None``.
+
+You can assign validators to a declared config option::
+
+  option = declaration.declare("ckanext.my_ext.option", True)
+  option.set_validators("not_missing boolean_validator")
+
+``set_validators`` accepts a string with the names of validators that must be applied to the config option.
+These validators need to registered in CKAN core or in your own extension using
+the :py:class:`~ckan.plugins.interfaces.IValidators` interface.
+
+.. note:: Declared default values are also passed to validators. In addition,
+          different validators can be applied to the same option multiple
+          times. This means that validators must be idempotent and that the
+          default value itself must be valid for the given set of validators.
+
+If you need to declare a lot of options, you can declare all of them at once loading a dict::
+
+  declaration.load_dict(DICT_WITH_DECLARATIONS)
+
+This allows to keep the configuration declaration in a separate file to make it easier to maintain if
+your plugin supports several config options.
+
+.. note:: ``declaration.load_dict()`` takes only python dictionary as
+          argument. If you store the declaration in an external file like a
+          JSON, YAML file, you have to parse it into a Python dictionary
+          yourself or use corresponding
+          :py:attr:`~ckan.plugins.toolkit.ckan.plugins.toolkit.blanket`. Read
+          the following section for additional information.
+
+
 Dynamic config options
 ^^^^^^^^^^^^^^^^^^^^^^