Merge pull request #16 from mam-dev/config-merge

Refactor Configuration use and add integration test

src/security_constraints/common.py

@@ -3,7 +3,13 @@
 import argparse
 import dataclasses
 import enum
-from typing import IO, Any, Dict, List, Optional, Set
+import sys
+from typing import IO, Any, Dict, List, Optional, Set, get_type_hints
+
+if sys.version_info >= (3, 8):
+    from typing import TypedDict
+else:
+    from typing_extensions import TypedDict
 
 
 class SeverityLevel(str, enum.Enum):
@@ -83,6 +89,12 @@ class ArgumentNamespace(argparse.Namespace):
     config: Optional[str]
     min_severity: SeverityLevel
 
+    def __setattr__(self, key, value):
+        # Makes it so that no attributes except those type hinted above can be set.
+        if key not in get_type_hints(self):
+            raise AttributeError(f"No attribute named '{key}'")
+        super().__setattr__(key, value)
+
 
 class SecurityConstraintsError(Exception):
     """Base class for all exceptions in this application."""
@@ -96,36 +108,62 @@ class FetchVulnerabilitiesError(SecurityConstraintsError):
     """Error which occurred when fetching vulnerabilities."""
 
 
-@dataclasses.dataclass
+@dataclasses.dataclass(frozen=True)
 class Configuration:
     """The application configuration.
 
-    Corresponds to the contents of a configuration file.
+    Corresponds to the contents of a configuration file,
+    or to (some of) the arguments given in a CLI execution.
 
     """
 
-    ignore_ids: List[str] = dataclasses.field(default_factory=list)
+    ignore_ids: Set[str] = dataclasses.field(default_factory=set)
     min_severity: SeverityLevel = dataclasses.field(default=SeverityLevel.CRITICAL)
 
-    def __post_init__(self) -> None:
-        # Type coerce the severity
-        self.min_severity = SeverityLevel(self.min_severity)
-
     def to_dict(self) -> Dict:
         def _dict_factory(data):
             def convert(obj):
                 if isinstance(obj, enum.Enum):
                     # Use values for Enums
                     return obj.value
+                if isinstance(obj, set):
+                    # Use ordered list for sets
+                    return sorted(obj)
                 return obj
 
             return dict((key, convert(value)) for key, value in data)
 
         return dataclasses.asdict(self, dict_factory=_dict_factory)
 
     @classmethod
-    def from_dict(cls, json: Dict) -> "Configuration":
-        return cls(**json)
+    def from_dict(cls, in_dict: Dict) -> "Configuration":
+        class Kwargs(TypedDict, total=False):
+            ignore_ids: Set[str]
+            min_severity: SeverityLevel
+
+        kwargs: Kwargs = {}
+        if "ignore_ids" in in_dict:
+            kwargs["ignore_ids"] = set(in_dict["ignore_ids"])
+        if "min_severity" in in_dict:
+            kwargs["min_severity"] = SeverityLevel(in_dict["min_severity"])
+        return cls(**kwargs)
+
+    @classmethod
+    def from_args(cls, args: ArgumentNamespace) -> "Configuration":
+        return cls(
+            ignore_ids=set(args.ignore_ids),
+            min_severity=args.min_severity,
+        )
+
+    @classmethod
+    def merge(cls, *config: "Configuration") -> "Configuration":
+        """Merge multiple Configurations into a new one."""
+        all_ignore_ids_entries = (c.ignore_ids for c in config)
+        all_min_severity_entries = (c.min_severity for c in config)
+        return cls(
+            ignore_ids=set.union(*all_ignore_ids_entries),
+            min_severity=min(all_min_severity_entries),
+        )
 
     @classmethod
     def supported_keys(cls) -> List[str]:

src/security_constraints/main.py

@@ -220,17 +220,16 @@ def get_args() -> ArgumentNamespace:
     return parser.parse_args(namespace=ArgumentNamespace())
 
 
-def get_config(config_file: Optional[str]) -> Configuration:
-    """Return configuration read from config_file.
+def get_config(args: ArgumentNamespace) -> Configuration:
+    """Return configuration read from args, including provided config file."""
+    config_from_args = Configuration.from_args(args)
 
-    Default config will be returned if config_file is None.
+    if args.config is None:
+        return config_from_args
 
-    """
-    if config_file is None:
-        return Configuration()
-
-    with open(config_file, mode="r") as fh:
-        return Configuration.from_dict(yaml.safe_load(fh))
+    with open(args.config, mode="r") as fh:
+        config_from_file = Configuration.from_dict(yaml.safe_load(fh))
+    return Configuration.merge(config_from_file, config_from_args)
 
 
 def setup_logging(debug: bool = False) -> None:
@@ -256,9 +255,7 @@ def main() -> int:
             raise AssertionError(
                 "'output' is not a stream! This suggests a programming error"
             )
-        config: Configuration = get_config(config_file=args.config)
-        config.ignore_ids.extend(sorted(args.ignore_ids))
-        config.min_severity = min(config.min_severity, args.min_severity)
+        config: Configuration = get_config(args=args)
 
         if args.dump_config:
             yaml.safe_dump(config.to_dict(), stream=sys.stdout)

test/conftest.py

@@ -0,0 +1,24 @@
+import pytest
+
+from security_constraints.common import ArgumentNamespace, SeverityLevel
+
+
+@pytest.fixture(name="arg_namespace")
+def fixture_arg_namespace() -> ArgumentNamespace:
+    return ArgumentNamespace(
+        dump_config=False,
+        debug=False,
+        version=False,
+        output=None,
+        ignore_ids=[],
+        config=None,
+        min_severity=SeverityLevel.CRITICAL,
+    )
+
+
+@pytest.fixture(name="github_token")
+def fixture_token_in_env(monkeypatch) -> str:
+    """Set SC_GITHUB_TOKEN environment variable and return it."""
+    token = "3e00409b-f017-4ecc-b7bf-f11f6e2a5693"
+    monkeypatch.setenv("SC_GITHUB_TOKEN", token)
+    return token

test/test_common.py

@@ -1,15 +1,17 @@
 from typing import List, Set
+from unittest.mock import Mock
 
 import pytest
 
 from security_constraints.common import (
+    ArgumentNamespace,
     Configuration,
     PackageConstraints,
     SecurityVulnerability,
     SeverityLevel,
 )
 
-IGNORE_IDS = ["A-1", "B-2"]
+IGNORE_IDS = {"A-1", "B-2"}
 
 
 @pytest.mark.parametrize("raw_severity", SeverityLevel.supported_values())
@@ -68,29 +70,118 @@ def test_get_higher_or_equal_severities(
     assert severity.get_higher_or_equal_severities() == expected
 
 
+def test_argument_namespace_can_be_modified(arg_namespace) -> None:
+    arg_namespace.dump_config = True
+    assert arg_namespace.dump_config
+    arg_namespace.debug = True
+    assert arg_namespace.debug
+    arg_namespace.version = True
+    assert arg_namespace.version
+    mock_output = Mock()
+    arg_namespace.output = mock_output
+    assert arg_namespace.output is mock_output
+    arg_namespace.ignore_ids = ["GHSA-X1"]
+    assert arg_namespace.ignore_ids == ["GHSA-X1"]
+    arg_namespace.config = "sc-conf.yaml"
+    assert arg_namespace.config == "sc-conf.yaml"
+    arg_namespace.min_severity = SeverityLevel.HIGH
+    assert arg_namespace.min_severity == SeverityLevel.HIGH
+
+
+def test_argument_namespace_cannot_be_extended(arg_namespace) -> None:
+    with pytest.raises(AttributeError):
+        arg_namespace.does_not_exist = True
+
+
 def test_configuration_to_dict() -> None:
     actual_dict = Configuration(ignore_ids=IGNORE_IDS).to_dict()
-    assert actual_dict == {"ignore_ids": IGNORE_IDS, "min_severity": "CRITICAL"}
+    assert actual_dict == {"ignore_ids": sorted(IGNORE_IDS), "min_severity": "CRITICAL"}
 
 
 def test_configuration_from_dict() -> None:
     created_from_dict = Configuration.from_dict(
-        {"ignore_ids": IGNORE_IDS, "min_severity": "HIGH"}
+        {"ignore_ids": list(IGNORE_IDS), "min_severity": "HIGH"}
     )
     assert created_from_dict == Configuration(
         ignore_ids=IGNORE_IDS, min_severity=SeverityLevel.HIGH
     )
+    assert isinstance(created_from_dict.ignore_ids, set)
     assert isinstance(created_from_dict.min_severity, SeverityLevel)
 
 
 def test_configuration_from_dict__no_min_severity_in_config() -> None:
-    created_from_dict = Configuration.from_dict({"ignore_ids": IGNORE_IDS})
+    created_from_dict = Configuration.from_dict({"ignore_ids": list(IGNORE_IDS)})
     assert created_from_dict == Configuration(
         ignore_ids=IGNORE_IDS, min_severity=SeverityLevel.CRITICAL
     )
     assert isinstance(created_from_dict.min_severity, SeverityLevel)
 
 
+def test_configuration_from_args() -> None:
+    created_from_args = Configuration.from_args(
+        ArgumentNamespace(
+            dump_config=False,
+            debug=False,
+            version=False,
+            output=Mock(),
+            ignore_ids=["GHSA-1", "GHSA-3"],
+            config=None,
+            min_severity=SeverityLevel.HIGH,
+        )
+    )
+    assert created_from_args == Configuration(
+        ignore_ids={"GHSA-1", "GHSA-3"},
+        min_severity=SeverityLevel.HIGH,
+    )
+
+
+@pytest.mark.parametrize(
+    "configs, expected",
+    [
+        ([Configuration()], Configuration()),
+        (
+            [
+                Configuration(min_severity=SeverityLevel.LOW),
+                Configuration(ignore_ids={"GHSA-3"}),
+            ],
+            Configuration(ignore_ids={"GHSA-3"}, min_severity=SeverityLevel.LOW),
+        ),
+        (
+            [
+                Configuration(ignore_ids={"GHSA-2"}),
+                Configuration(ignore_ids={"GHSA-3"}),
+            ],
+            Configuration(ignore_ids={"GHSA-3", "GHSA-2"}),
+        ),
+        (
+            [
+                Configuration(
+                    ignore_ids={"GHSA-1", "GHSA-6", "GHSA-2"},
+                    min_severity=SeverityLevel.CRITICAL,
+                ),
+                Configuration(
+                    ignore_ids={"GHSA-2", "GHSA-4"}, min_severity=SeverityLevel.CRITICAL
+                ),
+                Configuration(
+                    ignore_ids={"GHSA-3", "GHSA-4"}, min_severity=SeverityLevel.HIGH
+                ),
+                Configuration(
+                    ignore_ids={"GHSA-5", "GHSA-1"}, min_severity=SeverityLevel.MODERATE
+                ),
+            ],
+            Configuration(
+                ignore_ids={"GHSA-1", "GHSA-2", "GHSA-3", "GHSA-4", "GHSA-5", "GHSA-6"},
+                min_severity=SeverityLevel.MODERATE,
+            ),
+        ),
+    ],
+)
+def test_configuration_merge(
+    configs: List[Configuration], expected: Configuration
+) -> None:
+    assert Configuration.merge(*configs) == expected
+
+
 def test_configuration_supported_keys() -> None:
     assert Configuration.supported_keys() == ["ignore_ids", "min_severity"]
 

test/test_github_security_advisory.py

@@ -11,14 +11,6 @@
 )
 
 
-@pytest.fixture(name="github_token")
-def fixture_token_in_env(monkeypatch) -> str:
-    """Set SC_GITHUB_TOKEN environment variable and return it."""
-    token = "3e00409b-f017-4ecc-b7bf-f11f6e2a5693"
-    monkeypatch.setenv("SC_GITHUB_TOKEN", token)
-    return token
-
-
 def test_instantiate_without_token_in_env() -> None:
     with pytest.raises(FailedPrerequisitesError):
         _ = GithubSecurityAdvisoryAPI()