Add link config validation

.pre-commit-config.yaml

@@ -38,6 +38,7 @@ repos:
           - "jinja2>=2.11.3"         # 3.1.2 / 3.1.2
           - "packaging>=20"          # 20 seems to be available with RHEL8
           - "pint>=0.16.1"           # 0.16.1
+          - "pydantic>=1.10, <2.0"
           - "pygments>=2.7.4"        # 2.7.4 is the current one available for RHEL9
           - "requests>=2.25.1"       # 2.28.2 / 2.31.0
           - "ruamel.yaml>=0.16.6"    # 0.17.32 / 0.17.32
@@ -86,6 +87,7 @@ repos:
           - "jinja2>=2.11.3"         # 3.1.2 / 3.1.2
           - "packaging>=20"          # 20 seems to be available with RHEL8
           - "pint>=0.16.1"           # 0.16.1 / 0.19.x  TODO: Pint 0.20 requires larger changes to tmt.hardware
+          - "pydantic>=1.10, <2.0"
           - "pygments>=2.7.4"        # 2.7.4 is the current one available for RHEL9
           - "requests>=2.25.1"       # 2.28.2 / 2.31.0
           - "ruamel.yaml>=0.16.6"    # 0.17.32 / 0.17.32

pyproject.toml

@@ -36,6 +36,7 @@ dependencies = [              # F39 / PyPI
     "jinja2>=2.11.3",         # 3.1.2 / 3.1.2
     "packaging>=20",          # 20 seems to be available with RHEL8
     "pint>=0.16.1",           # 0.16.1
+    "pydantic>=1.10, <2.0",
     "pygments>=2.7.4",        # 2.7.4 is the current one available for RHEL9
     "requests>=2.25.1",       # 2.28.2 / 2.31.0
     "ruamel.yaml>=0.16.6",    # 0.17.32 / 0.17.32

tests/unit/test_config.py

@@ -0,0 +1,134 @@
+import queue
+import re
+import textwrap
+import threading
+import unittest
+import unittest.mock
+from unittest.mock import MagicMock
+
+import fmf
+import pytest
+
+import tmt.config
+from tmt.utils import Path
+
+
+@pytest.fixture
+def config_path(tmppath: Path, monkeypatch) -> Path:
+    config_path = tmppath / 'config'
+    config_path.mkdir()
+    monkeypatch.setattr(tmt.config, 'effective_config_dir', MagicMock(return_value=config_path))
+    return config_path
+
+
+def test_config(config_path: Path):
+    """ Config smoke test """
+    run = Path('/var/tmp/tmt/test')
+    config1 = tmt.config.Config()
+    config1.last_run = run
+    config2 = tmt.config.Config()
+    assert config2.last_run.resolve() == run.resolve()
+
+
+def test_last_run_race(tmppath: Path, monkeypatch):
+    """ Race in last run symlink shouldn't be fatal """
+    config_path = tmppath / 'config'
+    config_path.mkdir()
+    monkeypatch.setattr(tmt.config, 'effective_config_dir', MagicMock(return_value=config_path))
+    mock_logger = unittest.mock.MagicMock()
+    monkeypatch.setattr(tmt.utils.log, 'warning', mock_logger)
+    config = tmt.config.Config()
+    results = queue.Queue()
+    threads = []
+
+    def create_last_run(config, counter):
+        try:
+            last_run_path = tmppath / f"run-{counter}"
+            last_run_path.mkdir()
+            val = config.last_run = last_run_path
+            results.put(val)
+        except Exception as err:
+            results.put(err)
+
+    total = 20
+    for i in range(total):
+        threads.append(threading.Thread(target=create_last_run, args=(config, i)))
+    for t in threads:
+        t.start()
+    for t in threads:
+        t.join()
+
+    all_good = True
+    for _ in threads:
+        value = results.get()
+        if isinstance(value, Exception):
+            # Print exception for logging
+            print(value)
+            all_good = False
+    assert all_good
+    # Getting into race is not certain, do not assert
+    # assert mock_logger.called
+    assert config.last_run, "Some run was stored as last run"
+
+
+def test_link_config_invalid(config_path: Path):
+    config_yaml = textwrap.dedent("""
+        issue-tracker:
+          - type: jiRA
+            url: invalid_url
+            tmt-web-url: https://
+            unknown: value
+        additional_key:
+          foo: bar
+        """).strip()
+    fmf.Tree.init(path=config_path)
+    (config_path / 'link.fmf').write_text(config_yaml)
+
+    with pytest.raises(tmt.utils.MetadataError) as error:
+        _ = tmt.config.Config().link
+
+    cause = str(error.value.__cause__)
+    assert '6 validation errors for LinkConfig' in cause
+    assert re.search(r'type\s*value is not a valid enumeration member', cause)
+    assert re.search(r'url\s*invalid or missing URL scheme', cause)
+    assert re.search(r'tmt-web-url\s*URL host invalid', cause)
+    assert re.search(r'unknown\s*extra fields not permitted', cause)
+    assert re.search(r'token\s*field required', cause)
+    assert re.search(r'additional_key\s*extra fields not permitted', cause)
+
+
+def test_link_config_valid(config_path: Path):
+    config_yaml = textwrap.dedent("""
+        issue-tracker:
+          - type: jira
+            url: https://issues.redhat.com
+            tmt-web-url: https://tmt-web-url.com
+            token: secret
+        """).strip()
+    fmf.Tree.init(path=config_path)
+    (config_path / 'link.fmf').write_text(config_yaml)
+
+    link = tmt.config.Config().link
+
+    assert link.issue_tracker[0].type == 'jira'
+    assert link.issue_tracker[0].url == 'https://issues.redhat.com'
+    assert link.issue_tracker[0].tmt_web_url == 'https://tmt-web-url.com'
+    assert link.issue_tracker[0].token == 'secret'
+
+
+def test_link_config_missing(config_path: Path):
+    fmf.Tree.init(path=config_path)
+
+    assert tmt.config.Config().link is None
+
+
+def test_link_config_empty(config_path: Path):
+    fmf.Tree.init(path=config_path)
+    (config_path / 'link.fmf').touch()
+
+    with pytest.raises(tmt.utils.SpecificationError) as error:
+        _ = tmt.config.Config().link
+
+    cause = str(error.value.__cause__)
+    assert '1 validation error for LinkConfig' in cause
+    assert re.search(r'issue-tracker\s*field required', cause)

tests/unit/test_utils.py

@@ -17,6 +17,7 @@
 import pytest
 
 import tmt
+import tmt.config
 import tmt.log
 import tmt.plugins
 import tmt.steps.discover
@@ -204,56 +205,6 @@ def test_inject_auth_git_url(monkeypatch) -> None:
         inject_auth_git_url('https://example.com/broken/something')
 
 
-def test_config():
-    """ Config smoke test """
-    run = Path('/var/tmp/tmt/test')
-    config1 = tmt.utils.Config()
-    config1.last_run = run
-    config2 = tmt.utils.Config()
-    assert config2.last_run.resolve() == run.resolve()
-
-
-def test_last_run_race(tmppath: Path, monkeypatch):
-    """ Race in last run symlink shouldn't be fatal """
-    config_path = tmppath / 'config'
-    config_path.mkdir()
-    monkeypatch.setattr(tmt.utils, 'effective_config_dir', MagicMock(return_value=config_path))
-    mock_logger = unittest.mock.MagicMock()
-    monkeypatch.setattr(tmt.utils.log, 'warning', mock_logger)
-    config = tmt.utils.Config()
-    results = queue.Queue()
-    threads = []
-
-    def create_last_run(config, counter):
-        try:
-            last_run_path = tmppath / f"run-{counter}"
-            last_run_path.mkdir()
-            val = config.last_run = last_run_path
-            results.put(val)
-        except Exception as err:
-            results.put(err)
-
-    total = 20
-    for i in range(total):
-        threads.append(threading.Thread(target=create_last_run, args=(config, i)))
-    for t in threads:
-        t.start()
-    for t in threads:
-        t.join()
-
-    all_good = True
-    for _ in threads:
-        value = results.get()
-        if isinstance(value, Exception):
-            # Print exception for logging
-            print(value)
-            all_good = False
-    assert all_good
-    # Getting into race is not certain, do not assert
-    # assert mock_logger.called
-    assert config.last_run, "Some run was stored as last run"
-
-
 def test_workdir_env_var(tmppath: Path, monkeypatch, root_logger):
     """ Test TMT_WORKDIR_ROOT environment variable """
     # Cannot use monkeypatch.context() as it is not present for CentOS Stream 8
@@ -1737,9 +1688,9 @@ def tearDown(self):
         shutil.rmtree(self.tmp)
 
     @unittest.mock.patch('jira.JIRA.add_simple_link')
-    @unittest.mock.patch('tmt.utils.Config')
+    @unittest.mock.patch('tmt.config.Config.fmf_tree', new_callable=unittest.mock.PropertyMock)
     def test_jira_link_test_only(self, mock_config_tree, mock_add_simple_link) -> None:
-        mock_config_tree.return_value.fmf_tree = self.config_tree
+        mock_config_tree.return_value = self.config_tree
         test = tmt.Tree(logger=self.logger, path=self.tmp).tests(names=['tmp/test'])[0]
         tmt.utils.jira.link(
             tmt_objects=[test],
@@ -1752,9 +1703,9 @@ def test_jira_link_test_only(self, mock_config_tree, mock_add_simple_link) -> No
         assert '&test-path=%2Ftests%2Funit%2Ftmp' in result['url']
 
     @unittest.mock.patch('jira.JIRA.add_simple_link')
-    @unittest.mock.patch('tmt.utils.Config')
+    @unittest.mock.patch('tmt.config.Config.fmf_tree', new_callable=unittest.mock.PropertyMock)
     def test_jira_link_test_plan_story(self, mock_config_tree, mock_add_simple_link) -> None:
-        mock_config_tree.return_value.fmf_tree = self.config_tree
+        mock_config_tree.return_value = self.config_tree
         test = tmt.Tree(logger=self.logger, path=self.tmp).tests(names=['tmp/test'])[0]
         plan = tmt.Tree(logger=self.logger, path=self.tmp).plans(names=['tmp'])[0]
         story = tmt.Tree(logger=self.logger, path=self.tmp).stories(names=['tmp'])[0]
@@ -1778,9 +1729,9 @@ def test_jira_link_test_plan_story(self, mock_config_tree, mock_add_simple_link)
         assert '&story-path=%2Ftests%2Funit%2Ftmp' in result['url']
 
     @unittest.mock.patch('jira.JIRA.add_simple_link')
-    @unittest.mock.patch('tmt.utils.Config')
+    @unittest.mock.patch('tmt.config.Config.fmf_tree', new_callable=unittest.mock.PropertyMock)
     def test_create_link_relation(self, mock_config_tree, mock_add_simple_link) -> None:
-        mock_config_tree.return_value.fmf_tree = self.config_tree
+        mock_config_tree.return_value = self.config_tree
         test = tmt.Tree(logger=self.logger, path=self.tmp).tests(names=['tmp/test'])[0]
         tmt.utils.jira.link(
             tmt_objects=[test],

tmt/base.py

@@ -36,6 +36,7 @@
 
 import tmt.base
 import tmt.checks
+import tmt.config
 import tmt.convert
 import tmt.export
 import tmt.frameworks
@@ -3363,7 +3364,7 @@ def __init__(self,
                  logger: tmt.log.Logger) -> None:
         """ Initialize tree, workdir and plans """
         # Use the last run id if requested
-        self.config = tmt.utils.Config()
+        self.config = tmt.config.Config()
 
         if cli_invocation is not None:
             if cli_invocation.options.get('last'):

tmt/cli.py

@@ -16,6 +16,7 @@
 
 import tmt
 import tmt.base
+import tmt.config
 import tmt.convert
 import tmt.export
 import tmt.identifier
@@ -2171,7 +2172,7 @@ def completion(**kwargs: Any) -> None:
 
 def setup_completion(shell: str, install: bool, context: Context) -> None:
     """ Setup completion based on the shell """
-    config = tmt.utils.Config()
+    config = tmt.config.Config()
     # Fish gets installed into its special location where it is automatically
     # loaded.
     if shell == 'fish':

tmt/config/__init__.py

@@ -0,0 +1,91 @@
+import functools
+import os
+from contextlib import suppress
+from typing import Optional, cast
+
+import fmf
+import fmf.utils
+from pydantic import ValidationError
+
+import tmt.utils
+from tmt._compat.pathlib import Path
+from tmt.config.models.link import LinkConfig
+
+# Config directory
+DEFAULT_CONFIG_DIR = Path('~/.config/tmt')
+
+
+def effective_config_dir() -> Path:
+    """
+    Find out what the actual config directory is.
+
+    If ``TMT_CONFIG_DIR`` variable is set, it is used. Otherwise,
+    :py:const:`DEFAULT_CONFIG_DIR` is picked.
+    """
+
+    if 'TMT_CONFIG_DIR' in os.environ:
+        return Path(os.environ['TMT_CONFIG_DIR']).expanduser()
+
+    return DEFAULT_CONFIG_DIR.expanduser()
+
+
+class Config:
+    """ User configuration """
+
+    def __init__(self) -> None:
+        """ Initialize config directory path """
+        self.path = effective_config_dir()
+        self.logger = tmt.utils.log
+
+        try:
+            self.path.mkdir(parents=True, exist_ok=True)
+        except OSError as error:
+            raise tmt.utils.GeneralError(
+                f"Failed to create config '{self.path}'.") from error
+
+    @property
+    def _last_run_symlink(self) -> Path:
+        return self.path / 'last-run'
+
+    @property
+    def last_run(self) -> Optional[Path]:
+        """ Get the last run workdir path """
+        return self._last_run_symlink.resolve() if self._last_run_symlink.is_symlink() else None
+
+    @last_run.setter
+    def last_run(self, workdir: Path) -> None:
+        """ Set the last run to the given run workdir """
+
+        with suppress(OSError):
+            self._last_run_symlink.unlink()
+
+        try:
+            self._last_run_symlink.symlink_to(workdir)
+        except FileExistsError:
+            # Race when tmt runs in parallel
+            self.logger.warning(
+                f"Unable to mark '{workdir}' as the last run, "
+                "'tmt run --last' might not pick the right run directory.")
+        except OSError as error:
+            raise tmt.utils.GeneralError(
+                f"Unable to save last run '{self.path}'.\n{error}")
+
+    @functools.cached_property
+    def fmf_tree(self) -> fmf.Tree:
+        """ Return the configuration tree """
+        try:
+            return fmf.Tree(self.path)
+        except fmf.utils.RootError as error:
+            raise tmt.utils.MetadataError(f"Config tree not found in '{self.path}'.") from error
+
+    @property
+    def link(self) -> Optional[LinkConfig]:
+        """ Return the link configuration, if present. """
+        link_config = cast(Optional[fmf.Tree], self.fmf_tree.find('/link'))
+        if not link_config:
+            return None
+        try:
+            return LinkConfig.parse_obj(link_config.data)
+        except ValidationError as error:
+            raise tmt.utils.SpecificationError(
+                f"Invalid link configuration in '{link_config.name}'.") from error

tmt/config/models/__init__.py

@@ -0,0 +1,13 @@
+from pydantic import BaseModel, Extra
+
+
+def create_alias(name: str) -> str:
+    return name.replace('_', '-')
+
+
+class BaseConfig(BaseModel):
+    class Config:
+        # Accept only keys with dashes instead of underscores
+        alias_generator = create_alias
+        extra = Extra.forbid
+        validate_assignment = True