CLI: Fail command if --config file contains unknown key (#5939)

Up till now, the `--config` option would accept configuration files with
keys that did not correspond to existing options on the command. This
would lead to incorrect options to be swallowed silently, which can be
surprising to users if they accidentally misspelled an option or used
one that didn't exist.

Here the callback is updated to check the specified keys in the config
and cross-reference them to the params that are defined on the command.
If any unknown keys are specified a `BadParameter` exception is raised
which displays which keys are not supported.

The implementation was originally provided by the `click-config-file`
dependency, however, in order to customize it with the desired behavior
we had to essentially vendor the entire code, since there was no
available hook for the customizations. The code is taken from
https://github.com/phha/click_config_file/blob/7b93a20b4c79458987fac116418859f30a16d82a
which corresponds to `v0.6.0`. Besides the main changes, the default
provider is changed to `yaml_config_file_provider`, the docstrings are
reformated and type hints are added.

Author: sphuber

aiida/cmdline/params/options/config.py

@@ -11,8 +11,19 @@
 """
 .. py:module::config
     :synopsis: Convenience class for configuration file option
+
+The functions :func:`configuration_callback` and :func:`configuration_option` were directly taken from the repository
+https://github.com/phha/click_config_file/blob/7b93a20b4c79458987fac116418859f30a16d82a/click_config_file.py with a
+minor modification to ``configuration_callback`` to add a check for unknown parameters in the configuration file and
+the default provider is changed to :func:`yaml_config_file_provider`.
 """
-import click_config_file
+from __future__ import annotations
+
+import functools
+import os
+import typing as t
+
+import click
 import yaml
 
 from .overridable import OverridableOption
@@ -25,9 +36,115 @@ def yaml_config_file_provider(handle, cmd_name):  # pylint: disable=unused-argum
     return yaml.safe_load(handle)
 
 
-class ConfigFileOption(OverridableOption):
+def configuration_callback(
+    cmd_name: str | None,
+    option_name: str,
+    config_file_name: str,
+    saved_callback: t.Callable[..., t.Any] | None,
+    provider: t.Callable[..., t.Any],
+    implicit: bool,
+    ctx: click.Context,
+    param: click.Parameter,
+    value: t.Any,
+):
+    """Callback for reading the config file.
+
+    Also takes care of calling user specified custom callback afterwards.
+
+    :param cmd_name: The command name. This is used to determine the configuration directory.
+    :param option_name: The name of the option. This is used for error messages.
+    :param config_file_name: The name of the configuration file.
+    :param saved_callback: User-specified callback to be called later.
+    :param provider: A callable that parses the configuration file and returns a dictionary of the configuration
+        parameters. Will be called as ``provider(file_path, cmd_name)``. Default: ``yaml_config_file_provider``.
+    :param implicit: Whether a implicit value should be applied if no configuration option value was provided.
+    :param ctx: ``click`` context.
+    :param param: ``click`` parameters.
+    :param value: Value passed to the parameter.
+    """
+    ctx.default_map = ctx.default_map or {}
+    cmd_name = cmd_name or ctx.info_name
+
+    if implicit:
+        default_value = os.path.join(click.get_app_dir(cmd_name), config_file_name)  # type: ignore[arg-type]
+        param.default = default_value
+        value = value or default_value
+
+    if value:
+        try:
+            config = provider(value, cmd_name)
+        except Exception as exception:
+            raise click.BadOptionUsage(option_name, f'Error reading configuration file: {exception}', ctx)
+
+        valid_params = [param.name for param in ctx.command.params if param.name != option_name]
+        specified_params = list(config.keys())
+        unknown_params = set(specified_params).difference(set(valid_params))
+
+        if unknown_params:
+            raise click.BadParameter(
+                f'Invalid configuration file, the following keys are not supported: {unknown_params}', ctx, param
+            )
+
+        ctx.default_map.update(config)
+
+    return saved_callback(ctx, param, value) if saved_callback else value
+
+
+def configuration_option(*param_decls, **attrs):
+    """Adds configuration file support to a click application.
+
+    This will create an option of type ``click.File`` expecting the path to a configuration file. When specified, it
+    overwrites the default values for all other click arguments or options with the corresponding value from the
+    configuration file. The default name of the option is ``--config``. By default, the configuration will be read from
+    a configuration directory as determined by ``click.get_app_dir``. This decorator accepts the same arguments as
+    ``click.option`` and ``click.Path``. In addition, the following keyword arguments are available:
+
+    :param cmd_name: str
+        The command name. This is used to determine the configuration directory. Default: ``ctx.info_name``.
+    :param config_file_name: str
+        The name of the configuration file. Default: ``config``.
+    :param implicit: bool
+        If ``True`` then implicitly create a value for the configuration option using the above parameters. If a
+        configuration file exists in this path it will be applied even if no configuration option was suppplied as a
+        CLI argument or environment variable. If ``False`` only apply a configuration file that has been explicitely
+        specified. Default: ``False``.
+    :param provider: callable
+        A callable that parses the configuration file and returns a dictionary of the configuration parameters. Will be
+        called as ``provider(file_path, cmd_name)``. Default: ``yaml_config_file_provider``.
     """
-    Wrapper around click_config_file.configuration_option that increases reusability.
+    param_decls = param_decls or ('--config',)
+    option_name = param_decls[0]
+
+    def decorator(func):
+        attrs.setdefault('is_eager', True)
+        attrs.setdefault('help', 'Read configuration from FILE.')
+        attrs.setdefault('expose_value', False)
+        implicit = attrs.pop('implicit', True)
+        cmd_name = attrs.pop('cmd_name', None)
+        config_file_name = attrs.pop('config_file_name', 'config')
+        provider = attrs.pop('provider', yaml_config_file_provider)
+        path_default_params = {
+            'exists': False,
+            'file_okay': True,
+            'dir_okay': False,
+            'writable': False,
+            'readable': True,
+            'resolve_path': False
+        }
+        path_params = {k: attrs.pop(k, v) for k, v in path_default_params.items()}
+        attrs['type'] = attrs.get('type', click.Path(**path_params))
+        saved_callback = attrs.pop('callback', None)
+        partial_callback = functools.partial(
+            configuration_callback, cmd_name, option_name, config_file_name, saved_callback, provider, implicit
+        )
+        attrs['callback'] = partial_callback
+        return click.option(*param_decls, **attrs)(func)
+
+    return decorator
+
+
+class ConfigFileOption(OverridableOption):
+    """Reusable option that reads a configuration file containing values for other command parameters.
 
     Example::
 
@@ -49,8 +166,7 @@ def computer_setup(computer_name):
     """
 
     def __init__(self, *args, **kwargs):
-        """
-        Store the default args and kwargs.
+        """Store the default args and kwargs.
 
         :param args: default arguments to be used for the option
         :param kwargs: default keyword arguments to be used that can be overridden in the call
@@ -59,8 +175,7 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
     def __call__(self, **kwargs):
-        """
-        Override the stored kwargs, (ignoring args as we do not allow option name changes) and return the option.
+        """Override the stored kwargs, (ignoring args as we do not allow option name changes) and return the option.
 
         :param kwargs: keyword arguments that will override those set in the construction
         :return: click_config_file.configuration_option constructed with args and kwargs defined during construction
@@ -69,4 +184,4 @@ def __call__(self, **kwargs):
         kw_copy = self.kwargs.copy()
         kw_copy.update(kwargs)
 
-        return click_config_file.configuration_option(*self.args, **kw_copy)
+        return configuration_option(*self.args, **kw_copy)

environment.yml

@@ -10,7 +10,6 @@ dependencies:
 - archive-path~=0.4.2
 - aio-pika~=6.6
 - circus~=0.18.0
-- click-config-file~=0.6.0
 - click-spinner~=0.1.8
 - click~=8.1
 - disk-objectstore~=0.6.0

pyproject.toml

@@ -29,7 +29,6 @@ dependencies = [
     "archive-path~=0.4.2",
     "aio-pika~=6.6",
     "circus~=0.18.0",
-    "click-config-file~=0.6.0",
     "click-spinner~=0.1.8",
     "click~=8.1",
     "disk-objectstore~=0.6.0",

requirements/requirements-py-3.10.txt

@@ -20,7 +20,6 @@ cffi==1.15.0
 charset-normalizer==2.0.8
 circus==0.18.0
 click==8.1.0
-click-config-file==0.6.0
 click-spinner==0.1.10
 configobj==5.0.6
 coverage==6.5.0

requirements/requirements-py-3.11.txt

@@ -22,7 +22,6 @@ cffi==1.15.1
 charset-normalizer==2.1.1
 circus==0.18.0
 click==8.1.3
-click-config-file==0.6.0
 click-spinner==0.1.10
 configobj==5.0.6
 contourpy==1.0.6

requirements/requirements-py-3.8.txt

@@ -20,7 +20,6 @@ cffi==1.15.0
 charset-normalizer==2.0.8
 circus==0.18.0
 click==8.1.0
-click-config-file==0.6.0
 click-spinner==0.1.10
 configobj==5.0.6
 coverage==6.5.0

requirements/requirements-py-3.9.txt

@@ -20,7 +20,6 @@ cffi==1.15.0
 charset-normalizer==2.0.8
 circus==0.18.0
 click==8.1.0
-click-config-file==0.6.0
 click-spinner==0.1.10
 configobj==5.0.6
 coverage==6.5.0

tests/cmdline/params/options/test_config.py

@@ -0,0 +1,63 @@
+# -*- coding: utf-8 -*-
+###########################################################################
+# Copyright (c), The AiiDA team. All rights reserved.                     #
+# This file is part of the AiiDA code.                                    #
+#                                                                         #
+# The code is hosted on GitHub at https://github.com/aiidateam/aiida-core #
+# For further information on the license, see the LICENSE.txt file        #
+# For further information please visit http://www.aiida.net               #
+###########################################################################
+# pylint: disable=redefined-outer-name
+"""Unit tests for the :class:`aiida.cmdline.params.options.config.ConfigOption`."""
+import functools
+import tempfile
+import textwrap
+
+import click
+import pytest
+
+from aiida.cmdline.params.options import CONFIG_FILE
+
+
+@pytest.fixture
+def run_cli_command(run_cli_command):
+    """Override the ``run_cli_command`` fixture to always run with ``use_subprocess=False`` for tests in this module."""
+    return functools.partial(run_cli_command, use_subprocess=False)
+
+
+@click.command()
+@click.option('--integer', type=int)
+@click.option('--boolean', type=bool)
+@CONFIG_FILE()
+def cmd(integer, boolean):
+    """Test command for :class:`aiida.cmdline.params.options.config.ConfigOption`."""
+    click.echo(f'Integer: {integer}')
+    click.echo(f'Boolean: {boolean}')
+
+
+def test_valid(run_cli_command):
+    """Test the option for a valid configuration file."""
+    with tempfile.NamedTemporaryFile('w+') as handle:
+        handle.write(textwrap.dedent("""
+            integer: 1
+            boolean: false
+        """))
+        handle.flush()
+
+        result = run_cli_command(cmd, ['--config', handle.name])
+        assert 'Integer: 1' in result.output_lines[0]
+        assert 'Boolean: False' in result.output_lines[1]
+
+
+def test_invalid_unknown_keys(run_cli_command):
+    """Test the option for an invalid configuration file containing unknown keys."""
+    with tempfile.NamedTemporaryFile('w+') as handle:
+        handle.write(textwrap.dedent("""
+            integer: 1
+            unknown: 2.0
+        """))
+        handle.flush()
+
+        result = run_cli_command(cmd, ['--config', handle.name], raises=True)
+        assert "Error: Invalid value for '--config': Invalid configuration file" in result.stderr
+        assert "the following keys are not supported: {'unknown'}" in result.stderr