From 8336bc32c82ef3e5f8b6b560dd36d084427b9218 Mon Sep 17 00:00:00 2001 From: Adam Souzis Date: Tue, 17 Dec 2024 16:21:25 -0800 Subject: [PATCH] parser: validate that deployment blueprint template overrides are compatible with the base topology's templates. --- tests/test_syntax.py | 40 ++++++++++++++++++++++++++++++++++++- tosca-parser | 2 +- unfurl/manifest-schema.json | 5 ++++- unfurl/manifest.py | 15 +++++++++++++- unfurl/merge.py | 14 ++++++++++--- unfurl/yamlmanifest.py | 2 +- 6 files changed, 70 insertions(+), 8 deletions(-) diff --git a/tests/test_syntax.py b/tests/test_syntax.py index 502ec42c..de7df34e 100755 --- a/tests/test_syntax.py +++ b/tests/test_syntax.py @@ -5,10 +5,11 @@ import pytest from unfurl.yamlmanifest import YamlManifest -from unfurl.util import UnfurlError +from unfurl.util import UnfurlError, UnfurlValidationError from unfurl.to_json import to_blueprint, to_deployment, node_type_to_graphql from unfurl.localenv import LocalEnv from unfurl.planrequests import _find_implementation +from toscaparser.common.exception import TypeMismatchError Atlas = "Atlas@github.com/onecommons/unfurl.git/tests/examples:include-json-ensemble" @@ -337,3 +338,40 @@ def test_property_default_null(self): the_app = root.find_instance("the_app") assert the_app assert the_app.attributes["null_default"] is None + + +def test_deployment_blueprint(): + dp_yaml = """ +apiVersion: unfurl/v1beta1 +kind: Ensemble +environment: + deployment_blueprint: test +spec: + service_template: + tosca_definitions_version: tosca_simple_unfurl_1_0_0 + node_types: + Node: + derived_from: tosca.nodes.Root + Derived: + derived_from: Node + Unrelated: + derived_from: tosca.nodes.Root + + topology_template: + node_templates: + node: + type: Node + node2: + type: Node + deployment_blueprints: + test: + node_templates: + node: + type: %s + """ + with pytest.raises(UnfurlValidationError, match='TypeMismatchError: node template "node" must be of type "Node". in node template "node"'): + ensemble = YamlManifest(dp_yaml % "Unrelated") + ensemble = YamlManifest(dp_yaml % "Derived") + assert ensemble.context["deployment_blueprint"] == "test" + assert ensemble.get_deployment_blueprints() + assert ensemble.get_root_resource() diff --git a/tosca-parser b/tosca-parser index b8e345f9..7e15ed4d 160000 --- a/tosca-parser +++ b/tosca-parser @@ -1 +1 @@ -Subproject commit b8e345f93ae6bc61006b4d77f01dc49124bcf5c2 +Subproject commit 7e15ed4d9f5186c27d2f6ef47636cacaab1560f4 diff --git a/unfurl/manifest-schema.json b/unfurl/manifest-schema.json index e613a9b2..f3c2bb51 100644 --- a/unfurl/manifest-schema.json +++ b/unfurl/manifest-schema.json @@ -680,7 +680,10 @@ "properties": { "apiVersion": { "type": "string", - "const": "unfurl/v1alpha1" + "enum": [ + "unfurl/v1alpha1", + "unfurl/v1beta1" + ] }, "kind": { "type": "string", diff --git a/unfurl/manifest.py b/unfurl/manifest.py index 46dc45a0..bb0298da 100644 --- a/unfurl/manifest.py +++ b/unfurl/manifest.py @@ -33,6 +33,7 @@ TopologyInstance, ) from .util import ( + API_VERSION, UnfurlError, assert_not_none, is_relative_to, @@ -140,6 +141,7 @@ def __init__(self, path: Optional[str], localEnv: Optional["LocalEnv"] = None): self.imports = Imports() self.imports.manifest = self self.modules: Optional[Dict] = None + self.apiVersion = API_VERSION def _add_repositories_from_environment(self) -> None: assert self.localEnv @@ -210,13 +212,24 @@ def _load_spec( repositoriesTpl[name] = value # make sure this is present - toscaDef["tosca_definitions_version"] = TOSCA_VERSION + if "tosca_definitions_version" not in toscaDef: + toscaDef["tosca_definitions_version"] = TOSCA_VERSION + if self.apiVersion == "unfurl/v1beta1": + # if overriding a template, make sure it is compatible with the old one by adding "should_implement" hint + def replaceStrategy(key, old, new): + old_type = old.get("type") + if old_type: + new.setdefault("metadata", {})["should_implement"] = old_type + return new + else: + replaceStrategy = "replace" # type: ignore if more_spec: # don't merge individual templates toscaDef = merge_dicts( toscaDef, more_spec, replaceKeys=["node_templates", "relationship_templates"], + replaceStrategy=replaceStrategy ) yaml_dict_cls = yaml_dict_type(bool(self.localEnv and self.localEnv.readonly)) if not isinstance(toscaDef, yaml_dict_cls): diff --git a/unfurl/merge.py b/unfurl/merge.py index 2a5080a6..f72987a0 100644 --- a/unfurl/merge.py +++ b/unfurl/merge.py @@ -2,7 +2,7 @@ # SPDX-License-Identifier: MIT import itertools import re -from typing import Any, Dict, List, MutableMapping, Optional, Tuple +from typing import Any, Callable, Dict, List, MutableMapping, Optional, Tuple, Union from collections import namedtuple from collections.abc import Mapping, MutableSequence, Sequence @@ -82,14 +82,16 @@ def factory(*args, **kws): # other values besides delete not supported because current code can leave those keys in final result mergeStrategyKey = "+%" # supported values: "whiteout", "nullout", "merge", "error" +MappingMergeStrategy = Union[str, Callable[[str, Mapping, Mapping], Mapping]] def merge_dicts( b: Mapping, a: Mapping, cls=None, replaceKeys=None, - defaultStrategy="merge", + defaultStrategy: MappingMergeStrategy="merge", listStrategy="append_unique", + replaceStrategy: MappingMergeStrategy="replace", ) -> dict: """ Returns a new dict (or cls) that recursively merges b into a. @@ -105,7 +107,7 @@ def merge_dicts( if key == mergeStrategyKey: continue if replaceKeys and key in replaceKeys: - childStrategy = "replace" + childStrategy = replaceStrategy else: childStrategy = "merge" # note: for merging treat None as an empty map @@ -126,6 +128,7 @@ def merge_dicts( cls, defaultStrategy=childStrategy, replaceKeys=replaceKeys, + replaceStrategy=replaceStrategy, listStrategy=listStrategy, ) continue @@ -133,6 +136,11 @@ def merge_dicts( raise UnfurlError( "merging %s is not allowed, +%%: error was set" % key ) + if callable(strategy): + if val is None: # empty map, treat as missing key + continue + cp[key] = strategy(key, bval, val) + continue # otherwise we ignore bval because key is already in a if strategy == "whiteout": skip.append(key) diff --git a/unfurl/yamlmanifest.py b/unfurl/yamlmanifest.py index 89b96a40..12b099c4 100644 --- a/unfurl/yamlmanifest.py +++ b/unfurl/yamlmanifest.py @@ -275,6 +275,7 @@ def __init__( if self.manifest.path: logger.debug("loaded ensemble manifest at %s", self.manifest.path) manifest = self.manifest.expanded + self.apiVersion = manifest.get("apiVersion") spec = manifest.get("spec", {}) self.context = manifest.get("environment", CommentedMap()) if localEnv: @@ -509,7 +510,6 @@ def _add_deployment_blueprint_template( resource_templates[template_name] = local_resource_templates[ template_name ] - if resource_templates: node_templates = more_spec["topology_template"]["node_templates"] self._load_resource_templates(resource_templates, node_templates, False)