diff --git a/netengine/config/loader.py b/netengine/config/loader.py index e376024..57a02e6 100644 --- a/netengine/config/loader.py +++ b/netengine/config/loader.py @@ -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]: @@ -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 @@ -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 diff --git a/netengine/spec/loader.py b/netengine/spec/loader.py index 56d11b6..db7334d 100644 --- a/netengine/spec/loader.py +++ b/netengine/spec/loader.py @@ -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 @@ -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( @@ -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: @@ -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 diff --git a/tests/test_config.py b/tests/test_config.py index 7a2f81f..aaed14d 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,5 +1,6 @@ """Tests for configuration management.""" +from dataclasses import dataclass, field from pathlib import Path from tempfile import TemporaryDirectory @@ -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, @@ -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.""" @@ -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.""" @@ -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.""" @@ -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.)."""