Read user-specific configuration if available

This makes using only the client CLI easier for end users. In the
course, refactor how configuration files are processed in duffy.cli.

Fixes: #432

Signed-off-by: Nils Philippsen <nils@redhat.com>

Author: nphilipp

duffy/cli.py

@@ -2,6 +2,7 @@
 import logging
 import sys
 from datetime import timedelta
+from pathlib import Path
 from typing import List, Optional, Tuple, Union
 
 import click
@@ -10,6 +11,7 @@
     import uvicorn
 except ImportError:  # pragma: no cover
     uvicorn = None
+import xdg.BaseDirectory
 import yaml
 
 try:
@@ -49,7 +51,8 @@
 from .util import UNSET, SentinelType
 from .version import __version__
 
-DEFAULT_CONFIG_FILE = "/etc/duffy"
+DEFAULT_CONFIG_LOCATIONS = ("/etc/duffy", f"{xdg.BaseDirectory.xdg_config_home}/duffy")
+DEFAULT_CONFIG_PATHS = tuple(Path(loc) for loc in DEFAULT_CONFIG_LOCATIONS)
 
 log = logging.getLogger(__name__)
 
@@ -119,17 +122,7 @@ def convert(self, value, param, ctx):
 NODES_SPEC = NodesSpecType()
 
 
-# Global setup and CLI options
-
-
-def init_config(ctx, param, filename):
-    ctx.ensure_object(dict)
-    try:
-        read_configuration(filename, clear=ctx.obj.get("clear_config", True), validate=False)
-    except FileNotFoundError:
-        if filename is not DEFAULT_CONFIG_FILE:
-            raise
-    ctx.obj["clear_config"] = False
+# CLI groups and commands
 
 
 @click.group(name="duffy")
@@ -146,26 +139,32 @@ def init_config(ctx, param, filename):
     default=None,
 )
 @click.option(
+    "config_paths",
     "-c",
     "--config",
-    type=click.Path(),
-    default=DEFAULT_CONFIG_FILE,
-    callback=init_config,
-    is_eager=True,
-    expose_value=False,
-    help="Read configuration from the specified YAML files or directories.",
-    show_default=True,
+    type=click.Path(exists=True),
+    multiple=True,
+    help=(
+        "Read configuration from the specified YAML files or directories instead of the default"
+        f" paths ({', '.join(DEFAULT_CONFIG_LOCATIONS)})"
+    ),
     metavar="FILE_OR_DIR",
 )
 @click.version_option(version=__version__, prog_name="Duffy")
 @click.pass_context
-def cli(ctx: click.Context, loglevel: Optional[str]):
+def cli(ctx: click.Context, loglevel: Optional[str], config_paths: Tuple[Path]):
     ctx.ensure_object(dict)
     ctx.obj["loglevel"] = loglevel
 
     logging.basicConfig(level=loglevel.upper() if isinstance(loglevel, str) else loglevel)
 
-    read_configuration(clear=False, validate=True)
+    if not config_paths:
+        # Ignore non-existent default paths
+        config_paths = tuple(path for path in DEFAULT_CONFIG_PATHS if path.exists())
+
+    log.debug(f"Reading configuration from: {', '.join(str(p) for p in config_paths)}")
+
+    read_configuration(*config_paths, clear=True, validate=True)
 
 
 # Check & dump configuration

duffy/configuration/main.py

@@ -1,7 +1,7 @@
 from copy import deepcopy
 from itertools import chain
 from pathlib import Path
-from typing import List, Union
+from typing import List, Sequence, Union
 
 import yaml
 
@@ -26,7 +26,7 @@ def _expand_normalize_config_files(config_files: List[Union[Path, str]]) -> List
 
 
 def read_configuration(
-    *config_files: List[Union[Path, str]], clear: bool = True, validate: bool = True
+    *config_files: Sequence[Union[Path, str]], clear: bool = True, validate: bool = True
 ):
     config_files = _expand_normalize_config_files(config_files)
 

poetry.lock

@@ -55,6 +55,7 @@ psycopg2 = {version = "^2.9.2", optional = true}
 aiodns = {version = "^3.0.0", optional = true}
 pydantic = ">=1.6.2"
 aiosqlite = {version = ">=0.17.0", optional = true}
+pyxdg = ">=0.27"
 
 [tool.poetry.dev-dependencies]
 Jinja2 = "^3.0.3"

pyproject.toml

@@ -2,47 +2,44 @@
 
 import csv
 import sys
+from pathlib import Path
 
 import click
+import xdg.BaseDirectory
 import yaml
 from sqlalchemy import select
 
 from duffy.configuration import read_configuration
 from duffy.database import init_sync_model, sync_session_maker
 from duffy.database.model import Tenant
 
-DEFAULT_CONFIG_FILE = "/etc/duffy"
+DEFAULT_CONFIG_LOCATIONS = ("/etc/duffy", f"{xdg.BaseDirectory.xdg_config_home}/duffy")
+DEFAULT_CONFIG_PATHS = tuple(Path(loc) for loc in DEFAULT_CONFIG_LOCATIONS)
 
 
 class dump_dialect(csv.unix_dialect):
     quotechar = "'"
 
 
-def init_config(ctx, param, filename):
-    ctx.ensure_object(dict)
-    try:
-        read_configuration(filename, clear=ctx.obj.get("clear_config", True), validate=False)
-    except FileNotFoundError:
-        if filename is not DEFAULT_CONFIG_FILE:
-            raise
-    ctx.obj["clear_config"] = False
-
-
 @click.group()
 @click.option(
+    "config_paths",
     "--config",
     "-c",
-    type=click.Path(),
-    default=DEFAULT_CONFIG_FILE,
-    callback=init_config,
-    is_eager=True,
-    expose_value=False,
-    help="Read configuration from the specified YAML files or directories.",
-    show_default=True,
+    type=click.Path(exists=True),
+    multiple=True,
+    help=(
+        "Read configuration from the specified YAML files or directories instead of the default"
+        f" paths ({', '.join(DEFAULT_CONFIG_LOCATIONS)})"
+    ),
     metavar="FILE_OR_DIR",
 )
-def cli():
-    read_configuration(clear=False, validate=True)
+def cli(config_paths):
+    if not config_paths:
+        # Ignore non-existent default paths
+        config_paths = tuple(path for path in DEFAULT_CONFIG_PATHS if path.exists())
+
+    read_configuration(*config_paths, clear=True, validate=True)
 
 
 def read_csv_files(users_file, userkeys_file):

scripts/import_csv.py

@@ -1,7 +1,6 @@
 import copy
 import logging
 from datetime import timedelta
-from tempfile import TemporaryDirectory
 from unittest import mock
 from uuid import uuid4
 
@@ -27,18 +26,10 @@ def runner():
 
 
 @pytest.fixture(autouse=True, scope="module")
-def dont_read_etc_duffy():
-    # Modify the default value for `--config`. For that, find the right parameter object on the
-    # (click-wrapped) cli() function, then mock its default below.
-    for param in cli.params:
-        if param.name == "config":
-            break
-    else:  # Oops, didn't find right param object. This shouldn't happen!
-        raise RuntimeError("Can't find right parameter object for `--config`.")
-
-    with TemporaryDirectory(prefix="dont_read_etc_duffy-") as tmpdir, mock.patch(
-        "duffy.cli.DEFAULT_CONFIG_FILE", new=tmpdir
-    ), mock.patch.object(param, "default", new=tmpdir):
+def dont_read_system_user_config():
+    with mock.patch("duffy.cli.DEFAULT_CONFIG_LOCATIONS", new=()), mock.patch(
+        "duffy.cli.DEFAULT_CONFIG_PATHS", new=()
+    ):
         yield
 
 
@@ -102,38 +93,6 @@ def test_convert(self, testcase):
             assert converted == {"pool": "test", "quantity": "1"}
 
 
-@pytest.mark.parametrize(
-    "testcase", ("default", "default-not-found", "other-not-found", "default-plus-one")
-)
-@mock.patch("duffy.cli.read_configuration")
-def test_init_config(read_configuration, testcase):
-    if "not-found" in testcase:
-        read_configuration.side_effect = FileNotFoundError()
-
-    if "default" in testcase:
-        expectation = noop_context()
-        filename = duffy.cli.DEFAULT_CONFIG_FILE
-    else:
-        expectation = pytest.raises(FileNotFoundError)
-        filename = "boop"
-
-    ctx = mock.MagicMock()
-    ctx.obj = {}
-    param = mock.MagicMock()
-
-    with expectation:
-        duffy.cli.init_config(ctx, param, filename)
-
-    read_configuration.assert_called_once_with(filename, clear=True, validate=False)
-
-    if "plus-one" in testcase:
-        read_configuration.reset_mock(return_value=True, side_effect=True)
-
-        duffy.cli.init_config(ctx, param, "foo")
-
-        read_configuration.assert_called_once_with("foo", clear=False, validate=False)
-
-
 def test_cli_version(runner):
     result = runner.invoke(cli, ["--version"])
     assert result.exit_code == 0
@@ -156,8 +115,7 @@ def test_cli_suggestion(runner):
 def test_cli_missing_config(tmp_path, runner):
     missing_config_file = tmp_path / "missing_duffy_config.yaml"
     result = runner.invoke(cli, [f"--config={missing_config_file.absolute()}"])
-    assert result.exit_code == 1
-    assert isinstance(result.exception, FileNotFoundError)
+    assert result.exit_code != 0
 
 
 @pytest.mark.duffy_config(example_config=True)