Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion netengine/config/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,15 @@ def parse_dotted_overrides(values: Iterable[str]) -> dict[str, Any]:


class ConfigLoader:
"""Load and merge OmegaConf configurations."""
"""Load and merge OmegaConf configurations.

Precedence is intentionally last-writer-wins for all helpers in this
module. The contract is: structured schema defaults < caller-provided
defaults < config/spec file data < explicit overrides such as CLI
``--set`` values. Nested mappings are deep-merged at each layer, so a
higher-precedence layer can replace one nested field without discarding
sibling fields from lower-precedence layers.
"""

@staticmethod
def load_yaml(path: Path | str) -> dict[str, Any]:
Expand All @@ -86,6 +94,10 @@ def load_config(
) -> T:
"""Load configuration with optional file and overrides.

Merge precedence is: structured schema defaults < ``defaults`` <
``config_file`` < ``overrides``. Later layers win for the same field,
while nested dictionaries are deep-merged.

Args:
config_schema: OmegaConf dataclass schema
defaults: Default configuration dictionary
Expand Down Expand Up @@ -117,6 +129,10 @@ def load_config(
def merge_configs(*configs: dict[str, Any] | Any) -> dict[str, Any]:
"""Merge multiple configurations in order.

Later positional arguments have higher precedence than earlier ones.
Nested dictionaries are deep-merged, allowing higher-precedence layers
to override individual nested fields while preserving siblings.

Args:
*configs: Configuration dictionaries to merge

Expand Down
53 changes: 25 additions & 28 deletions netengine/spec/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,37 +218,25 @@ def _validate_spec_data(
return spec


def load_spec(yaml_path: str | Path, *, validate_feature_states: bool = True) -> NetEngineSpec:
"""Load and validate a NetEngine YAML specification.

Args:
data: Composed raw specification dictionary
validate_feature_states: Whether to enforce feature-state metadata

Returns:
Validated, immutable NetEngineSpec object
def validate_spec_data(
data: dict[str, Any], *, validate_feature_states: bool = True
) -> NetEngineSpec:
"""Validate composed raw spec data and return a ``NetEngineSpec``.

Raises:
SpecLoadError: If the composed spec data is invalid
Callers should compose data according to the project precedence contract
before validation: structured model defaults < base spec <
environment/spec file < explicit overrides/CLI ``--set``.
"""
yaml_path = Path(yaml_path)

if not yaml_path.exists():
raise SpecLoadError(f"Spec file not found: {yaml_path}")

try:
with open(yaml_path) as f:
data = yaml.safe_load(f)
except yaml.YAMLError as e:
raise SpecLoadError(f"Failed to parse YAML: {e}")
except IOError as e:
raise SpecLoadError(f"Failed to read file: {e}")

return _validate_spec_data(data, validate_feature_states=validate_feature_states)


def load_spec(yaml_path: str | Path, *, validate_feature_states: bool = True) -> NetEngineSpec:
"""Load and validate a NetEngine YAML specification.
"""Load and validate a single NetEngine YAML specification.

A single file is applied over structured ``NetEngineSpec`` defaults during
validation. Composition helpers use the broader precedence contract:
structured defaults < base spec < environment/spec file < explicit
overrides/CLI ``--set``.

Args:
yaml_path: Path to YAML spec file
Expand All @@ -272,7 +260,7 @@ def load_spec(yaml_path: str | Path, *, validate_feature_states: bool = True) ->
except IOError as e:
raise SpecLoadError(f"Failed to read file: {e}")

return validate_spec_data(data, validate_feature_states=validate_feature_states)
return _validate_spec_data(data, validate_feature_states=validate_feature_states)


def load_spec_with_composition(
Expand All @@ -283,10 +271,14 @@ def load_spec_with_composition(
) -> NetEngineSpec:
"""Load spec with optional base spec composition and overrides.

Supports merging a base spec with environment-specific overrides.
Precedence is: structured ``NetEngineSpec`` defaults < ``base_path`` <
``yaml_path`` < ``overrides`` (including CLI ``--set`` values). Nested
mappings are deep-merged, and later layers win for fields set in multiple
places.

Example:
base spec: spec.base.yaml
environment override: spec.prod.yaml
environment/spec override: spec.prod.yaml
inline overrides: {"logging": {"level": "ERROR"}}

Args:
Expand Down Expand Up @@ -334,6 +326,11 @@ def load_spec_with_environment(
) -> NetEngineSpec:
"""Load base spec and merge with environment-specific overrides.

Precedence is: structured ``NetEngineSpec`` defaults < ``base_spec`` <
``spec.{environment}.yaml`` when present < ``overrides`` (including CLI
``--set`` values). Nested mappings are deep-merged, and later layers win
for fields set in multiple places.

Automatically loads environment-specific spec file if it exists.
Pattern: spec.base.yaml or spec.yaml + spec.{environment}.yaml

Expand Down
102 changes: 102 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Tests for configuration management."""

from dataclasses import dataclass, field
from pathlib import Path
from tempfile import TemporaryDirectory

Expand All @@ -8,6 +9,7 @@

from netengine.config.loader import ConfigLoader, ConfigOverrideError, parse_dotted_overrides
from netengine.config.spec_config import SpecConfig
from netengine.cli.main import _load_spec_for_cli
from netengine.spec.loader import (
SpecLoadError,
load_spec,
Expand All @@ -17,6 +19,18 @@
from netengine.spec.models import NetEngineSpec


@dataclass
class _NestedConfig:
environment: str = "structured"
owner: str = "schema-owner"


@dataclass
class _PrecedenceConfig:
name: str = "schema"
metadata: _NestedConfig = field(default_factory=_NestedConfig)


@pytest.fixture
def temp_spec_dir() -> Path:
"""Create temporary directory for test specs."""
Expand Down Expand Up @@ -252,6 +266,36 @@ def test_merge_multiple_configs(self) -> None:
assert merged["c"] == 5
assert merged["d"] == 6

def test_load_config_precedence_contract(self, temp_spec_dir: Path) -> None:
"""Explicit overrides should win over files, defaults, and structured defaults."""
config_file = temp_spec_dir / "config.yaml"
with open(config_file, "w") as f:
yaml.dump(
{"name": "file", "metadata": {"environment": "file", "owner": "file-owner"}},
f,
)

config = ConfigLoader.load_config(
_PrecedenceConfig,
defaults={"name": "defaults", "metadata": {"environment": "defaults"}},
config_file=config_file,
overrides={"metadata": {"environment": "override"}},
)

assert config.name == "file"
assert config.metadata.environment == "override"
assert config.metadata.owner == "file-owner"

def test_merge_configs_nested_precedence_contract(self) -> None:
"""Later merge layers should win nested conflicts while preserving siblings."""
merged = ConfigLoader.merge_configs(
{"metadata": {"environment": "base", "name": "base-name"}},
{"metadata": {"environment": "env"}},
{"metadata": {"environment": "override"}},
)

assert merged["metadata"] == {"environment": "override", "name": "base-name"}


class TestSpecConfig:
"""Tests for spec configuration loading."""
Expand Down Expand Up @@ -364,6 +408,20 @@ def test_load_environment_with_overrides(

assert spec["metadata"]["environment"] == "prod"

def test_environment_precedence_with_nested_merge(
self, base_spec_file: Path, dev_spec_file: Path
) -> None:
"""Environment files should override base fields and explicit overrides should win last."""
overrides = {"metadata": {"environment": "cli"}}

spec = SpecConfig.load_environment_variants(
base_spec_file, environment="dev", overrides=overrides
)

assert spec["metadata"]["environment"] == "cli"
assert spec["metadata"]["name"] == "test-network"
assert spec["metadata"]["version"] == "1.0"


class TestSpecLoaderIntegration:
"""Integration tests for spec loader with OmegaConf."""
Expand Down Expand Up @@ -439,6 +497,50 @@ def test_load_spec_with_environment_rejects_unsupported_schema_version(
with pytest.raises(SpecLoadError, match="Unsupported spec metadata.schema_version"):
load_spec_with_environment(base_file, environment="dev")

def test_load_spec_with_environment_precedence_nested_merge(
self, temp_spec_dir: Path
) -> None:
"""Validated environment loading should apply base < env file < overrides."""
base = _minimal_spec()
base["metadata"]["environment"] = "base"
base["gateway_portal"]["real_internet"] = {"mode": "isolated"}
base_file = self._write_spec(temp_spec_dir, "spec.base.yaml", base)
self._write_spec(
temp_spec_dir,
"spec.dev.yaml",
{
"metadata": {"environment": "dev"},
"gateway_portal": {"real_internet": {"mode": "exposed"}},
},
)

spec = load_spec_with_environment(
base_file,
environment="dev",
overrides={"metadata": {"environment": "cli"}},
)

assert spec.metadata.environment == "cli"
assert spec.metadata.name == "test-network"
assert spec.gateway_portal.real_internet.mode.value == "exposed"

def test_load_spec_for_cli_env_plus_set_precedence(self, temp_spec_dir: Path) -> None:
"""CLI loading should apply --set values after --env composition."""
base = _minimal_spec()
base["metadata"]["environment"] = "base"
base_file = self._write_spec(temp_spec_dir, "spec.base.yaml", base)
self._write_spec(temp_spec_dir, "spec.dev.yaml", {"metadata": {"environment": "dev"}})

spec = _load_spec_for_cli(
str(base_file),
environment="dev",
set_values=("metadata.environment=cli", "gateway_portal.real_internet.mode=exposed"),
)

assert spec.metadata.environment == "cli"
assert spec.metadata.name == "test-network"
assert spec.gateway_portal.real_internet.mode.value == "exposed"


class TestSpecCrossValidation:
"""Tests for cross-field spec validation (CIDR overlap, name uniqueness, etc.)."""
Expand Down