From 4d3f5bdf472bd1854ae5b69ef5b2fc8dcc7b719c Mon Sep 17 00:00:00 2001 From: Paymaun Heidari Date: Wed, 15 Apr 2026 12:18:46 -0700 Subject: [PATCH] feat(siteops): support site.properties in parameter file paths Extend resolve_parameter_path() to resolve {{ site.properties. }} templates in manifest parameter file paths. Supports nested dot-separated paths (e.g., {{ site.properties.aioVersion }}). Enables version-driven parameter files where site config selects the version and the manifest dynamically resolves the corresponding file: parameters: - "parameters/aio-versions/{{ site.properties.aioVersion }}.yaml" Also fixes a pre-existing gap in validate() where parameter paths containing {{ }} templates were checked as literal file paths. Validation now resolves dynamic paths per-site and validates file existence, YAML content, and output references at the same depth as static paths. --- siteops/models.py | 14 ++ siteops/orchestrator.py | 57 +++++++- tests/test_models.py | 67 +++++++++ tests/test_orchestrator_validation.py | 187 ++++++++++++++++++++++++++ 4 files changed, 318 insertions(+), 7 deletions(-) diff --git a/siteops/models.py b/siteops/models.py index 3cdf742..5d5d115 100644 --- a/siteops/models.py +++ b/siteops/models.py @@ -626,6 +626,7 @@ def resolve_parameter_path(self, param_path: str, site: "Site") -> str: - {{ site.resourceGroup }} - Site resource group - {{ site.subscription }} - Site subscription - {{ site.labels. }} - Site label value + - {{ site.properties. }} - Site property value (nested paths supported) Args: param_path: Parameter file path with optional template variables @@ -643,4 +644,17 @@ def resolve_parameter_path(self, param_path: str, site: "Site") -> str: for key, value in site.labels.items(): result = result.replace(f"{{{{ site.labels.{key} }}}}", value) + # Resolve {{ site.properties. }} templates + for match in re.finditer(r"\{\{\s*site\.properties\.(\S+?)\s*\}\}", result): + prop_path = match.group(1) + value = site.properties + for part in prop_path.split("."): + if isinstance(value, dict) and part in value: + value = value[part] + else: + value = None + break + if value is not None: + result = result.replace(match.group(0), str(value)) + return result diff --git a/siteops/orchestrator.py b/siteops/orchestrator.py index a8f4e1d..16f4386 100644 --- a/siteops/orchestrator.py +++ b/siteops/orchestrator.py @@ -1705,14 +1705,30 @@ def validate(self, manifest_path: Path, selector: str | None = None) -> list[str # Validate manifest-level parameter files for param_path in manifest.parameters: - full_path = (self.workspace / param_path).resolve() - if not full_path.exists(): - errors.append(f"Manifest parameter file not found: {param_path}") + if "{{" in param_path: + # Dynamic path — validate resolved path for each site + for site in sites: + resolved = manifest.resolve_parameter_path(param_path, site) + full_path = (self.workspace / resolved).resolve() + if not full_path.exists(): + errors.append( + f"Manifest parameter file not found: {resolved} " + f"(resolved from '{param_path}' for site '{site.name}')" + ) + else: + try: + self.load_parameters(full_path) + except Exception as e: + errors.append(f"Invalid manifest parameter file {resolved}: {e}") else: - try: - self.load_parameters(full_path) - except Exception as e: - errors.append(f"Invalid manifest parameter file {param_path}: {e}") + full_path = (self.workspace / param_path).resolve() + if not full_path.exists(): + errors.append(f"Manifest parameter file not found: {param_path}") + else: + try: + self.load_parameters(full_path) + except Exception as e: + errors.append(f"Invalid manifest parameter file {param_path}: {e}") # Build step name lookup for output reference validation all_step_names = {step.name for step in manifest.steps} @@ -1747,6 +1763,33 @@ def validate(self, manifest_path: Path, selector: str | None = None) -> list[str continue for param_path in step.parameters: + if "{{" in param_path: + # Dynamic path — validate resolved path for each site + for site in sites: + resolved = manifest.resolve_parameter_path(param_path, site) + full_path = (self.workspace / resolved).resolve() + if not full_path.exists(): + errors.append( + f"Parameter file not found: {resolved} " + f"(step: {step.name}, resolved from '{param_path}' for site '{site.name}')" + ) + else: + try: + params = self.load_parameters(full_path) + errors.extend( + self._validate_output_references( + params, + step.name, + prior_step_names, + all_step_names, + resolved, + None, + ) + ) + except Exception as e: + errors.append(f"Invalid parameter file {resolved}: {e}") + continue + full_path = (self.workspace / param_path).resolve() if not full_path.exists(): errors.append(f"Parameter file not found: {param_path} (step: {step.name})") diff --git a/tests/test_models.py b/tests/test_models.py index d48fa6b..0c1d26c 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -940,6 +940,73 @@ def test_resolve_parameter_path_all_variables(self): result = manifest.resolve_parameter_path(path, site) assert result == "westus/rg-prod/sub-456.yaml" + def test_resolve_parameter_path_with_properties(self): + """Test {{ site.properties. }} resolution in parameter file paths.""" + manifest = Manifest(name="test", description="", sites=[], steps=[]) + site = Site( + name="munich-dev", + subscription="sub-123", + resource_group="rg-dev", + location="eastus", + properties={"aioVersion": "2603"}, + ) + + result = manifest.resolve_parameter_path( + "parameters/aio-versions/{{ site.properties.aioVersion }}.yaml", + site, + ) + assert result == "parameters/aio-versions/2603.yaml" + + def test_resolve_parameter_path_with_nested_properties(self): + """Test nested property path resolution.""" + manifest = Manifest(name="test", description="", sites=[], steps=[]) + site = Site( + name="munich-dev", + subscription="sub-123", + resource_group="rg-dev", + location="eastus", + properties={"config": {"variant": "standard"}}, + ) + + result = manifest.resolve_parameter_path( + "parameters/{{ site.properties.config.variant }}/defaults.yaml", + site, + ) + assert result == "parameters/standard/defaults.yaml" + + def test_resolve_parameter_path_with_missing_property(self): + """Unresolvable property path should leave template as-is.""" + manifest = Manifest(name="test", description="", sites=[], steps=[]) + site = Site( + name="munich-dev", + subscription="sub-123", + resource_group="rg-dev", + location="eastus", + properties={}, + ) + + path = "parameters/{{ site.properties.nonexistent }}/defaults.yaml" + result = manifest.resolve_parameter_path(path, site) + assert result == path + + def test_resolve_parameter_path_mixed_templates(self): + """Test mixing site.properties with other template variables.""" + manifest = Manifest(name="test", description="", sites=[], steps=[]) + site = Site( + name="munich-dev", + subscription="sub-123", + resource_group="rg-dev", + location="eastus", + labels={"environment": "dev"}, + properties={"aioVersion": "2603"}, + ) + + result = manifest.resolve_parameter_path( + "parameters/{{ site.labels.environment }}/{{ site.properties.aioVersion }}.yaml", + site, + ) + assert result == "parameters/dev/2603.yaml" + class TestSiteProperties: """Tests for Site properties field.""" diff --git a/tests/test_orchestrator_validation.py b/tests/test_orchestrator_validation.py index 13bccba..3be226f 100644 --- a/tests/test_orchestrator_validation.py +++ b/tests/test_orchestrator_validation.py @@ -533,6 +533,193 @@ def test_validate_duplicate_step_names(self, complete_workspace): assert "deploy-infra" in dup_errors[0] + def test_validate_dynamic_parameter_path_resolved(self, complete_workspace): + """Validation should resolve {{ site.properties.* }} in parameter paths.""" + orchestrator = Orchestrator(complete_workspace) + + # Create a site with a property and a matching parameter file + site_data = { + "name": "test-site", + "subscription": "sub-123", + "resourceGroup": "rg-test", + "location": "eastus", + "properties": {"variant": "standard"}, + } + (complete_workspace / "sites" / "test-site.yaml").write_text(yaml.dump(site_data)) + + # Create the version-specific parameter file + variant_dir = complete_workspace / "parameters" / "variants" + variant_dir.mkdir(parents=True, exist_ok=True) + (variant_dir / "standard.yaml").write_text(yaml.dump({"someParam": "value"})) + + manifest_data = { + "name": "dynamic-path-test", + "sites": ["test-site"], + "steps": [ + { + "name": "deploy", + "template": "templates/test.bicep", + "parameters": [ + "parameters/variants/{{ site.properties.variant }}.yaml", + ], + }, + ], + } + manifest_path = complete_workspace / "manifests" / "dynamic-path.yaml" + manifest_path.write_text(yaml.dump(manifest_data)) + + errors = orchestrator.validate(manifest_path) + param_errors = [e for e in errors if "variants" in e] + assert param_errors == [], f"Dynamic path should resolve: {param_errors}" + + def test_validate_dynamic_parameter_path_missing_file(self, complete_workspace): + """Validation should report missing files for resolved dynamic paths.""" + orchestrator = Orchestrator(complete_workspace) + + site_data = { + "name": "test-site-missing", + "subscription": "sub-123", + "resourceGroup": "rg-test", + "location": "eastus", + "properties": {"variant": "nonexistent"}, + } + (complete_workspace / "sites" / "test-site-missing.yaml").write_text(yaml.dump(site_data)) + + manifest_data = { + "name": "dynamic-path-missing", + "sites": ["test-site-missing"], + "steps": [ + { + "name": "deploy", + "template": "templates/test.bicep", + "parameters": [ + "parameters/variants/{{ site.properties.variant }}.yaml", + ], + }, + ], + } + manifest_path = complete_workspace / "manifests" / "dynamic-path-missing.yaml" + manifest_path.write_text(yaml.dump(manifest_data)) + + errors = orchestrator.validate(manifest_path) + param_errors = [e for e in errors if "nonexistent" in e] + assert len(param_errors) == 1 + assert "test-site-missing" in param_errors[0] + + def test_validate_dynamic_manifest_level_parameter_path(self, complete_workspace): + """Validation should resolve dynamic paths in manifest-level parameters.""" + orchestrator = Orchestrator(complete_workspace) + + site_data = { + "name": "test-site-manifest-dyn", + "subscription": "sub-123", + "resourceGroup": "rg-test", + "location": "eastus", + "properties": {"variant": "standard"}, + } + (complete_workspace / "sites" / "test-site-manifest-dyn.yaml").write_text(yaml.dump(site_data)) + + variant_dir = complete_workspace / "parameters" / "variants" + variant_dir.mkdir(parents=True, exist_ok=True) + (variant_dir / "standard.yaml").write_text(yaml.dump({"someParam": "value"})) + + manifest_data = { + "name": "manifest-dyn-path", + "sites": ["test-site-manifest-dyn"], + "parameters": [ + "parameters/variants/{{ site.properties.variant }}.yaml", + ], + "steps": [ + { + "name": "deploy", + "template": "templates/test.bicep", + }, + ], + } + manifest_path = complete_workspace / "manifests" / "manifest-dyn.yaml" + manifest_path.write_text(yaml.dump(manifest_data)) + + errors = orchestrator.validate(manifest_path) + param_errors = [e for e in errors if "variants" in e] + assert param_errors == [], f"Manifest-level dynamic path should resolve: {param_errors}" + + def test_validate_dynamic_parameter_path_invalid_yaml(self, complete_workspace): + """Validation should report invalid YAML in resolved dynamic parameter files.""" + orchestrator = Orchestrator(complete_workspace) + + site_data = { + "name": "test-site-bad-yaml", + "subscription": "sub-123", + "resourceGroup": "rg-test", + "location": "eastus", + "properties": {"variant": "broken"}, + } + (complete_workspace / "sites" / "test-site-bad-yaml.yaml").write_text(yaml.dump(site_data)) + + variant_dir = complete_workspace / "parameters" / "variants" + variant_dir.mkdir(parents=True, exist_ok=True) + (variant_dir / "broken.yaml").write_text("{ invalid yaml: [unclosed") + + manifest_data = { + "name": "dyn-path-bad-yaml", + "sites": ["test-site-bad-yaml"], + "steps": [ + { + "name": "deploy", + "template": "templates/test.bicep", + "parameters": [ + "parameters/variants/{{ site.properties.variant }}.yaml", + ], + }, + ], + } + manifest_path = complete_workspace / "manifests" / "dyn-bad-yaml.yaml" + manifest_path.write_text(yaml.dump(manifest_data)) + + errors = orchestrator.validate(manifest_path) + yaml_errors = [e for e in errors if "Invalid" in e and "broken" in e] + assert len(yaml_errors) == 1 + + def test_validate_dynamic_parameter_path_checks_output_refs(self, complete_workspace): + """Validation should check output references in resolved dynamic parameter files.""" + orchestrator = Orchestrator(complete_workspace) + + site_data = { + "name": "test-site-outref", + "subscription": "sub-123", + "resourceGroup": "rg-test", + "location": "eastus", + "properties": {"variant": "with-refs"}, + } + (complete_workspace / "sites" / "test-site-outref.yaml").write_text(yaml.dump(site_data)) + + variant_dir = complete_workspace / "parameters" / "variants" + variant_dir.mkdir(parents=True, exist_ok=True) + (variant_dir / "with-refs.yaml").write_text(yaml.dump({ + "someId": "{{ steps.nonexistent-step.outputs.id }}" + })) + + manifest_data = { + "name": "dyn-path-outref", + "sites": ["test-site-outref"], + "steps": [ + { + "name": "deploy", + "template": "templates/test.bicep", + "parameters": [ + "parameters/variants/{{ site.properties.variant }}.yaml", + ], + }, + ], + } + manifest_path = complete_workspace / "manifests" / "dyn-outref.yaml" + manifest_path.write_text(yaml.dump(manifest_data)) + + errors = orchestrator.validate(manifest_path) + ref_errors = [e for e in errors if "nonexistent-step" in e] + assert len(ref_errors) >= 1 + + class TestKubectlValidation: """Tests for kubectl step validation."""