launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+423533@xxxxxxxxxxxxxxxxxx>]

Jürgen Gmach has proposed merging ~jugmac00/lpcraft:add-modular-config into lpcraft:main.

Commit message:
Add modular plugin configuration

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jugmac00/lpcraft/+git/lpcraft/+merge/423533
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/lpcraft:add-modular-config into lpcraft:main.

diff --git a/NEWS.rst b/NEWS.rst
index bcec6ce..3624a23 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -9,6 +9,7 @@ Version history
 
 - Add ``lpcraft_execute_before_run`` and ``lpcraft_execute_after_run`` hooks.
 
+- Add support for pydantic configuration on plugin classes.
 
 0.0.14 (2022-05-18)
 ===================
diff --git a/docs/configuration.rst b/docs/configuration.rst
index 5e02c2b..ecaa727 100644
--- a/docs/configuration.rst
+++ b/docs/configuration.rst
@@ -82,6 +82,10 @@ Job definitions
     removing the ``matrix`` key itself, and updating it with the contents of
     each item in turn.
 
+.. note::
+
+    Plugins can define :ref:`plugin_configuration_keys`.
+
 .. _output-properties:
 
 Output properties
diff --git a/docs/plugins.rst b/docs/plugins.rst
index f150687..a8adda5 100644
--- a/docs/plugins.rst
+++ b/docs/plugins.rst
@@ -47,6 +47,42 @@ Available hooks
 .. automodule:: lpcraft.plugin.hookspecs
    :members:
 
+.. _plugin_configuration_keys:
+
+Additional configuration keys
+*****************************
+
+Plugins can have their own configuration keys.
+
+.. code-block:: python
+
+    @register(name="miniconda")
+    class MiniCondaPlugin(BasePlugin):
+        class Config(BaseConfig):
+            conda_packages: Optional[List[StrictStr]]
+            conda_python: Optional[StrictStr]
+
+The above code defines the ``MiniCondaPlugin`` with two additional configuration
+keys.
+
+These keys could be used in the ``.launchpad.yaml`` configuration file as
+following:
+
+.. code-block:: yaml
+
+    jobs:
+        myjob:
+            plugin: miniconda
+            conda-packages:
+                - mamba
+                - numpy=1.17
+                - scipy
+                - pip
+            conda-python: 3.8
+            run: |
+                pip install --upgrade pytest
+                python -m build .
+
 
 Builtin plugins
 ---------------
diff --git a/lpcraft/config.py b/lpcraft/config.py
index 06f6e23..0a4120d 100644
--- a/lpcraft/config.py
+++ b/lpcraft/config.py
@@ -5,12 +5,14 @@ import re
 from datetime import timedelta
 from enum import Enum
 from pathlib import Path
-from typing import Any, Dict, List, Optional, Union
+from typing import Any, Dict, List, Optional, Type, Union
 
 import pydantic
 from pydantic import StrictStr
 
 from lpcraft.errors import ConfigurationError
+from lpcraft.plugins import PLUGINS
+from lpcraft.plugins.plugins import BaseConfig, BasePlugin
 from lpcraft.utils import load_yaml
 
 
@@ -58,6 +60,22 @@ class Output(ModelConfigDefaults):
         return v
 
 
+def _validate_plugin_config(
+    plugin: Type[BasePlugin],
+    values: Dict[StrictStr, Any],
+    job_fields: List[str],
+) -> Dict[StrictStr, Any]:
+    plugin_config = {}
+    for k in plugin.Config.schema()["properties"].keys():
+        # configuration key belongs to the plugin
+        if k in values and k not in job_fields:
+            # TODO: should some error be raised if a plugin tries consuming
+            # a job configuration key?
+            plugin_config[k] = values.pop(k)
+    values["plugin-config"] = plugin.Config.parse_obj(plugin_config)
+    return values
+
+
 class Job(ModelConfigDefaults):
     """A job definition."""
 
@@ -75,6 +93,7 @@ class Job(ModelConfigDefaults):
     snaps: Optional[List[StrictStr]]
     packages: Optional[List[StrictStr]]
     plugin: Optional[StrictStr]
+    plugin_config: Optional[BaseConfig]
 
     @pydantic.validator("architectures", pre=True)
     def validate_architectures(
@@ -84,6 +103,23 @@ class Job(ModelConfigDefaults):
             v = [v]
         return v
 
+    @pydantic.root_validator(pre=True)
+    def move_plugin_config_settings(
+        cls, values: Dict[StrictStr, Any]
+    ) -> Dict[StrictStr, Any]:
+        """Delegate plugin settings to the plugin."""
+        if "plugin" in values:
+            base_values = values.copy()
+            if values["plugin"] not in PLUGINS:
+                raise ConfigurationError("Unknown plugin")
+            plugin = PLUGINS[values["plugin"]]
+            return _validate_plugin_config(
+                plugin=plugin,
+                values=base_values,
+                job_fields=list(cls.__fields__.keys()),
+            )
+        return values
+
 
 def _expand_job_values(
     values: Dict[StrictStr, Any]
diff --git a/lpcraft/plugin/manager.py b/lpcraft/plugin/manager.py
index d78f306..f6dfdd3 100644
--- a/lpcraft/plugin/manager.py
+++ b/lpcraft/plugin/manager.py
@@ -1,7 +1,6 @@
 import pluggy
 
 from lpcraft.config import Job
-from lpcraft.errors import ConfigurationError
 from lpcraft.plugin import NAME, hookspecs
 from lpcraft.plugin.lib import InternalPlugins
 from lpcraft.plugins import PLUGINS
@@ -16,8 +15,6 @@ def get_plugin_manager(job: Job) -> pluggy.PluginManager:
 
     # register builtin plugins
     if job.plugin:
-        if job.plugin not in PLUGINS:
-            raise ConfigurationError("Unknown plugin")
         pm.register(PLUGINS[job.plugin](job))
 
     return pm
diff --git a/lpcraft/plugin/tests/test_plugins.py b/lpcraft/plugin/tests/test_plugins.py
index cda49e4..6976849 100644
--- a/lpcraft/plugin/tests/test_plugins.py
+++ b/lpcraft/plugin/tests/test_plugins.py
@@ -5,13 +5,19 @@ import os
 import subprocess
 from pathlib import Path, PosixPath
 from textwrap import dedent
+from typing import List, Optional, Union, cast
 from unittest.mock import ANY, Mock, call, patch
 
 from craft_providers.lxd import launch
 from fixtures import TempDir
+from pydantic import StrictStr, validator
 
+import lpcraft.config
 from lpcraft.commands.tests import CommandBaseTestCase
 from lpcraft.errors import ConfigurationError
+from lpcraft.plugin.manager import get_plugin_manager
+from lpcraft.plugins import register
+from lpcraft.plugins.plugins import BaseConfig, BasePlugin
 from lpcraft.providers.tests import makeLXDProvider
 
 
@@ -245,3 +251,76 @@ class TestPlugins(CommandBaseTestCase):
             ],
             execute_run.call_args_list,
         )
+
+    def test_plugin_config_raises_notimplementederror(self):
+        config = dedent(
+            """
+            pipeline:
+                - build
+
+            jobs:
+                build:
+                    series: focal
+                    architectures: amd64
+                    plugin: pyproject-build
+        """
+        )
+        config_path = Path(".launchpad.yaml")
+        config_path.write_text(config)
+        config_obj = lpcraft.config.Config.load(config_path)
+        self.assertEqual(config_obj.jobs["build"][0].plugin, "pyproject-build")
+        pm = get_plugin_manager(config_obj.jobs["build"][0])
+        plugins = pm.get_plugins()
+        build_plugin = [
+            _
+            for _ in plugins
+            if _.__class__.__name__ == "PyProjectBuildPlugin"
+        ]
+        # build_plugin does not define its own configuration
+        self.assertRaises(
+            NotImplementedError, build_plugin[0].get_plugin_config
+        )
+
+    def test_plugin_config_sets_values(self):
+        config = dedent(
+            """
+            pipeline:
+                - build
+
+            jobs:
+                build:
+                    series: focal
+                    architectures: amd64
+                    plugin: fake-plugin
+                    python-version: 3.8
+        """
+        )
+        config_path = Path(".launchpad.yaml")
+        config_path.write_text(config)
+
+        @register(name="fake-plugin")
+        class FakePlugin(BasePlugin):
+            class Config(BaseConfig):
+                series: Optional[List[StrictStr]]
+                python_version: Optional[StrictStr]
+
+                @validator("python_version", pre=True)
+                def validate_python_version(
+                    cls, v: Union[str, float, int]
+                ) -> str:
+                    return str(v)
+
+            def get_plugin_config(self) -> "FakePlugin.Config":
+                return cast(FakePlugin.Config, self.config.plugin_config)
+
+        config_obj = lpcraft.config.Config.load(config_path)
+        job = config_obj.jobs["build"][0]
+        pm = get_plugin_manager(job)
+        plugins = pm.get_plugins()
+        fake_plugin = [
+            _ for _ in plugins if _.__class__.__name__ == "FakePlugin"
+        ]
+        self.assertEqual(job.plugin, "fake-plugin")
+        plugin_config = fake_plugin[0].get_plugin_config()
+        self.assertEqual(plugin_config.python_version, "3.8")
+        self.assertIsNone(getattr(plugin_config, "series", None))
diff --git a/lpcraft/plugins/plugins.py b/lpcraft/plugins/plugins.py
index 880816e..fc136ff 100644
--- a/lpcraft/plugins/plugins.py
+++ b/lpcraft/plugins/plugins.py
@@ -1,17 +1,47 @@
 # Copyright 2021 Canonical Ltd.  This software is licensed under the
 # GNU General Public License version 3 (see the file LICENSE).
 
-from __future__ import annotations
+from __future__ import annotations  # isort:skip
 
 __all__ = ["ToxPlugin", "PyProjectBuildPlugin"]
 
-from lpcraft.config import Job
+from typing import TYPE_CHECKING
+
+import pydantic
+
 from lpcraft.plugin import hookimpl
 from lpcraft.plugins import register
 
+# XXX: techalchemy 2022-03-25: prevent circular import of Job class
+if TYPE_CHECKING:
+
+    from lpcraft.config import Job  # pragma: no cover
+
+
+class BaseConfig(
+    pydantic.BaseModel,
+    extra=pydantic.Extra.forbid,
+    frozen=True,
+    alias_generator=lambda s: s.replace("_", "-"),
+    underscore_attrs_are_private=True,
+):
+    """Base config for plugin models."""
+
+
+class BasePlugin:
+    class Config(BaseConfig):
+        pass
+
+    def __init__(self, config: Job) -> None:
+        self.config = config
+
+    def get_plugin_config(self) -> BaseConfig:
+        """Return the properly typecast plugin configuration."""
+        raise NotImplementedError
+
 
 @register(name="tox")
-class ToxPlugin:
+class ToxPlugin(BasePlugin):
     """Installs `tox` and executes the configured environments.
 
     Usage:
@@ -19,9 +49,6 @@ class ToxPlugin:
         within the job definition.
     """
 
-    def __init__(self, config: Job) -> None:
-        self.config = config
-
     @hookimpl  # type: ignore
     def lpcraft_install_packages(self) -> list[str]:
         return ["python3-pip"]
@@ -41,7 +68,7 @@ class ToxPlugin:
 
 
 @register(name="pyproject-build")
-class PyProjectBuildPlugin:
+class PyProjectBuildPlugin(BasePlugin):
     """Installs `build` and builds a Python package according to PEP 517.
 
     Usage:
@@ -49,9 +76,6 @@ class PyProjectBuildPlugin:
         `pyproject-build` within the job definition.
     """
 
-    def __init__(self, config: Job) -> None:
-        self.config = config
-
     @hookimpl  # type: ignore
     def lpcraft_install_packages(self) -> list[str]:
         # Ubuntu 20.04 does not provide a packaged version of build,