Repository: kytta/bumerge (language: python)

Issue:
Validate variant and version
Each Butane config must have a variant and a version. bumerge should check it's there. If supplied via CLI, it should check that the valid one is specified and add one if it's not there.


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5150e95..ae581f4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,9 +2,17 @@
 SPDX-FileCopyrightText: © 2023 Nikita Karamov <me@kytta.dev>
 SPDX-License-Identifier: CC-BY-4.0 OR BSD-2-Clause
 -->
+<!-- markdownlint-disable MD024 -->
 
 # bumerge changelog
 
+## Unreleased
+
+### Added
+
+- [#6](https://github.com/kytta/bumerge/pull/6):
+  Validation of the `version` and `variant` fields
+
 ## [0.1.0] - 2023-04-28
 
 ### Added
diff --git a/bumerge.py b/bumerge.py
index 26ed351..a0ed052 100644
--- a/bumerge.py
+++ b/bumerge.py
@@ -21,9 +21,12 @@
 """
 from __future__ import annotations
 
+import sys
 from pathlib import Path
 from typing import TYPE_CHECKING
 from typing import Any
+from typing import Literal
+from typing import TypeVar
 
 if TYPE_CHECKING:
     from collections.abc import Iterable
@@ -37,6 +40,86 @@
 # TODO: use a proper schema (with validation?)
 JSONDict = dict[str, Any]
 
+# TODO: support other distributions
+Variant = Literal["fcos"]
+Version = str
+
+
+class ConfigurationError(BaseException):
+    """Base exception class for configuration errors."""
+
+    def __init__(self, message: str = "", *, field: str | None = None) -> None:
+        """Create a configuration error instance.
+
+        :param message: message that describes the error
+        :param field: configuration field that caused the error
+        """
+        super().__init__()
+        self.message = message
+        self.field = field
+
+    def __str__(self) -> str:
+        if self.field:
+            prefix = f"Error in field '{self.field}'"
+        else:
+            prefix = "Configuration error"
+
+        return f"{prefix}: {self.message}"
+
+
+class FieldRequiredError(ConfigurationError):
+    """Exception class for missing fields."""
+
+    def __init__(self, field: str, flag: str | None = None) -> None:
+        """Create a missing field error instance.
+
+        :param field: name of the field that is missing
+        :param flag: name of the CLI flag to set the default field value
+        """
+        super().__init__(
+            "Cannot infer field, and no default was given. Set the field in "
+            f"one of the configuration files, or use the '--{flag or field}' "
+            "command-line option.",
+            field=field,
+        )
+
+
+class FieldMismatchError(ConfigurationError):
+    """Exception class for mismatched fields.
+
+    These exception happen when the field defined in the config doesn't
+    match the one defined on the command line.
+    """
+
+    T = TypeVar("T")
+
+    def __init__(
+        self,
+        field: str,
+        expected: T,
+        actual: T,
+        flag: str | None = None,
+    ) -> None:
+        """Create a field mismatch error instance.
+
+        :param field:
+            name of the field with the incorrect value
+        :param expected:
+            the expected field value, that is, the one set via
+            the command-line argument
+        :param actual:
+            the actual field value, that is, the one set in the Butane
+            file
+        :param flag:
+            name of the CLI flag to set the default field value
+        """
+        super().__init__(
+            f"The configuration value {actual!r} does not match the value of "
+            f"the --{flag or field} flag: {expected!r}. Fix either the "
+            "configuration file or the flag.",
+            field=field,
+        )
+
 
 def merge_dicts(source: JSONDict, destination: JSONDict) -> JSONDict:
     """Merge one JSON-valid dictionary into another.
@@ -73,12 +156,66 @@ def read_config_files(config_files: Iterable[Path]) -> list[JSONDict]:
     yaml = YAML(typ="safe", pure=True)
     result = []
     for file in config_files:
-        with file.open() as fp:
-            result.append(yaml.load(fp))
+        try:
+            with file.open() as fp:
+                result.append(yaml.load(fp))
+        except OSError as exc:
+            raise ConfigurationError(str(exc)) from exc
 
     return result
 
 
+def validate_config(
+    config: JSONDict,
+    variant: Variant | None,
+    version: Version | None,
+) -> JSONDict:
+    """Validate the configuration.
+
+    This method will check the values of "variant" and "version" that
+    are required in Butane configuration files. If possible, it will add
+    the missing values.
+
+    :param config:
+        the configuration to be validated
+    :param variant:
+        the expected Butane specification
+    :param version:
+        the expected specification version
+    """
+    if "variant" not in config:
+        if variant is None:
+            raise FieldRequiredError(
+                field="variant",
+                flag="variant",
+            )
+        config["variant"] = variant
+    elif variant is not None and config["variant"] != variant:
+        raise FieldMismatchError(
+            field="variant",
+            expected=variant,
+            actual=config["variant"],
+            flag="variant",
+        )
+
+    if "version" not in config:
+        if version is None:
+            raise FieldRequiredError(
+                field="version",
+                flag="spec-version",
+            )
+        config["version"] = version
+    elif version is not None and config["version"] != version:
+        raise FieldMismatchError(
+            field="version",
+            expected=version,
+            actual=config["version"],
+            flag="spec-version",
+        )
+
+    return config
+
+
 def _main(argv: Sequence[str] | None = None) -> None:
     import argparse
 
@@ -97,6 +234,18 @@ def _main(argv: Sequence[str] | None = None) -> None:
         help="output file. Outputs to stdout by default",
         metavar="FILE",
     )
+    parser.add_argument(
+        "--variant",
+        default=None,
+        choices=["fcos"],
+        help="Butane specification variant",
+    )
+    parser.add_argument(
+        "--spec-version",
+        default=None,
+        choices=["1.5.0"],
+        help="Butane specification version",
+    )
     parser.add_argument(
         "files",
         nargs="+",
@@ -108,8 +257,18 @@ def _main(argv: Sequence[str] | None = None) -> None:
 
     result: JSONDict = {}
 
-    for config in read_config_files(args.files):
-        merge_dicts(config, result)
+    try:
+        for config in read_config_files(args.files):
+            merge_dicts(config, result)
+    except ConfigurationError as exc:
+        sys.stderr.write(f"{exc}\n")
+        raise SystemExit(1) from exc
+
+    try:
+        result = validate_config(result, args.variant, args.spec_version)
+    except ConfigurationError as exc:
+        sys.stderr.write(f"{exc}\n")
+        raise SystemExit(1) from exc
 
     yaml = YAML(typ="safe", pure=True)
     yaml.default_flow_style = False
diff --git a/pyproject.toml b/pyproject.toml
index 5480a3a..239c9a6 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -143,6 +143,7 @@ select = [
 ]
 extend-ignore = [
     "ANN101",  # mypy knows what 'self' is
+    "D105",  # docstrings for magic methods
 ]
 unfixable = ["ERA001"]
 


Test patch:
diff --git a/tests/bumerge_test.py b/tests/bumerge_test.py
index 46b691e..24d15ab 100644
--- a/tests/bumerge_test.py
+++ b/tests/bumerge_test.py
@@ -2,6 +2,7 @@
 # SPDX-License-Identifier: BSD-2-Clause
 from __future__ import annotations
 
+from contextlib import nullcontext
 from typing import TYPE_CHECKING
 
 import pytest
@@ -27,6 +28,24 @@ def test_main(capsys, argv, output):
     assert output in captured.out
 
 
+@pytest.mark.parametrize(
+    ("file", "printed"), [
+        ("users.bu", "Error in field"),
+        ("idontexist.bu", "Configuration error:"),
+    ],
+)
+def test_main_bad_config(
+        capsys,
+        shared_datadir: Path,
+        file: str,
+        printed: str):
+    with pytest.raises(SystemExit):
+        bumerge._main([str(shared_datadir / file)])
+
+    captured = capsys.readouterr()
+    assert printed in captured.err
+
+
 def test_main_with_files(capsys, shared_datadir: Path):
     files = [
         shared_datadir / "root.bu",
@@ -174,3 +193,28 @@ def test_read_config_files(shared_datadir: Path):
             },
         },
     ]
+
+
+@pytest.mark.parametrize(
+    ("config", "variant", "version", "context", "result"), [
+        ({}, None, None, pytest.raises(bumerge.FieldRequiredError), None),
+        ({"version": "1.5.0"}, None, None, pytest.raises(
+            bumerge.FieldRequiredError), None),
+        ({"variant": "fcos"}, None, None, pytest.raises(
+            bumerge.FieldRequiredError), None),
+        ({"version": "1.5.0"}, "fcos", "1.5.0", nullcontext(),
+         {"version": "1.5.0", "variant": "fcos"}),
+        ({"variant": "fcos"}, "fcos", "1.5.0", nullcontext(),
+         {"variant": "fcos", "version": "1.5.0"}),
+        ({"version": "1.4.0", "variant": "fcos"}, "fcos", "1.5.0",
+         pytest.raises(bumerge.FieldMismatchError), None),
+        ({"version": "1.5.0", "variant": "openshift"}, "fcos", "1.5.0",
+         pytest.raises(bumerge.FieldMismatchError), None),
+    ],
+)
+def test_validate_config(config, variant, version, context, result):
+    with context:
+        retval = bumerge.validate_config(config, variant, version)
+
+    if result:
+        assert retval == result