Merge pull request #1100 from projectsyn/fix/multi-version-verification

Fix component multi-version verification

Author: simu

commodore/compile.py

@@ -236,7 +236,7 @@ def setup_compile_environment(config: Config) -> tuple[dict[str, Any], Iterable[
     create_component_library_aliases(config, cluster_parameters)
 
     # Verify that all aliased components support instantiation
-    config.verify_component_aliases(cluster_parameters)
+    config.verify_component_aliases(inventory)
     config.register_component_deprecations(cluster_parameters)
     # Raise exception if component version override without URL is present in the
     # hierarchy.

commodore/component/compile.py

@@ -82,7 +82,7 @@ def compile_component(
 
         # Verify component alias
         nodes = kapitan_inventory(config)
-        config.verify_component_aliases(nodes[instance_name]["parameters"])
+        config.verify_component_aliases(nodes, bootstrap_target=instance_name)
 
         cluster_params = nodes[instance_name]["parameters"]
         create_component_library_aliases(config, cluster_params)

commodore/config.py

@@ -379,7 +379,12 @@ def get_component_aliases(self):
     def register_component_aliases(self, aliases: dict[str, str]):
         self._component_aliases.update(aliases)
 
-    def verify_component_aliases(self, cluster_parameters: dict):
+    def verify_component_aliases(
+        self, inventory: dict, bootstrap_target: Optional[str] = None
+    ):
+        if bootstrap_target is None:
+            bootstrap_target = self.inventory.bootstrap_target
+        cluster_parameters = inventory[bootstrap_target]["parameters"]
         for alias, cn in self._component_aliases.items():
             if alias != cn:
                 if not _component_is_aliasable(cluster_parameters, cn):
@@ -391,8 +396,9 @@ def verify_component_aliases(self, cluster_parameters: dict):
                 alias_has_version = (
                     cv.get("url") is not None or cv.get("version") is not None
                 )
+                ap = inventory.get(alias, {}).get("parameters", {})
                 if alias_has_version and not _component_supports_alias_version(
-                    cluster_parameters, cn, alias
+                    cluster_parameters, ap, cn, alias
                 ):
                     raise click.ClickException(
                         f"Component {cn} with alias {alias} does not support overriding instance version."
@@ -467,13 +473,14 @@ def _component_is_aliasable(cluster_parameters: dict, component_name: str):
 
 def _component_supports_alias_version(
     cluster_parameters: dict,
+    alias_parameters: dict,
     component_name: str,
     alias: str,
 ):
     ckey = component_parameters_key(component_name)
     cmeta = cluster_parameters[ckey].get("_metadata", {})
-    akey = component_parameters_key(alias)
-    ameta = cluster_parameters.get(akey, {}).get("_metadata", {})
+    # NOTE(sg): We access the merged alias parameters, so we need to lookup the component key.
+    ameta = alias_parameters[ckey].get("_metadata", {})
     return cmeta.get("multi_version", False) and ameta.get("multi_version", False)
 
 

tests/test_config.py

@@ -24,38 +24,51 @@
 from commodore.multi_dependency import dependency_key
 
 
+def make_inventory(cluster_params: dict) -> dict:
+    return {"cluster": {"parameters": cluster_params}}
+
+
 def test_verify_component_aliases_no_instance(config):
     alias_data = {"bar": "bar"}
     config.register_component_aliases(alias_data)
-    params = {"bar": {"namespace": "syn-bar"}}
+    params = make_inventory({"bar": {"namespace": "syn-bar"}})
 
     config.verify_component_aliases(params)
 
 
 def test_verify_component_aliases_explicit_no_instance(config):
     alias_data = {"bar": "bar"}
     config.register_component_aliases(alias_data)
-    params = {"bar": {"_metadata": {"multi_instance": False}, "namespace": "syn-bar"}}
+    params = make_inventory(
+        {"bar": {"_metadata": {"multi_instance": False}, "namespace": "syn-bar"}}
+    )
 
     config.verify_component_aliases(params)
 
 
 def test_verify_component_aliases_explicit_no_multiversion_exception(config):
     alias_data = {"baz": "bar"}
     config.register_component_aliases(alias_data)
-    params = {
-        "components": {
-            "bar": {"url": "foo", "version": "v1.0.0"},
-            "baz": {"version": "v1.1.0"},
-        },
-        "bar": {
-            "_metadata": {"multi_instance": True, "multi_version": False},
-            "namespace": "syn-bar",
-        },
-        "baz": {
-            "_metadata": {"multi_instance": True, "multi_version": False},
-            "namespace": "syn-baz",
-        },
+    params = make_inventory(
+        {
+            "components": {
+                "bar": {"url": "foo", "version": "v1.0.0"},
+                "baz": {"version": "v1.1.0"},
+            },
+            "bar": {
+                "_metadata": {"multi_instance": True, "multi_version": False},
+                "namespace": "syn-bar",
+            },
+        }
+    )
+    # Simulate merged target params for `baz` -> parameters key is `bar`
+    params["baz"] = {
+        "parameters": {
+            "bar": {
+                "_metadata": {"multi_instance": True, "multi_version": False},
+                "namespace": "syn-baz",
+            },
+        }
     }
 
     with pytest.raises(click.ClickException) as e:
@@ -70,19 +83,26 @@ def test_verify_component_aliases_explicit_no_multiversion_exception(config):
 def test_verify_component_aliases_explicit_no_multiversion_in_alias_exception(config):
     alias_data = {"baz": "bar"}
     config.register_component_aliases(alias_data)
-    params = {
-        "components": {
-            "bar": {"url": "foo", "version": "v1.0.0"},
-            "baz": {"version": "v1.1.0"},
-        },
-        "bar": {
-            "_metadata": {"multi_instance": True, "multi_version": True},
-            "namespace": "syn-bar",
-        },
-        "baz": {
-            "_metadata": {"multi_instance": True, "multi_version": False},
-            "namespace": "syn-baz",
-        },
+    params = make_inventory(
+        {
+            "components": {
+                "bar": {"url": "foo", "version": "v1.0.0"},
+                "baz": {"version": "v1.1.0"},
+            },
+            "bar": {
+                "_metadata": {"multi_instance": True, "multi_version": True},
+                "namespace": "syn-bar",
+            },
+        }
+    )
+    # Simulate merged target params for `baz` -> parameters key is `bar`
+    params["baz"] = {
+        "parameters": {
+            "bar": {
+                "_metadata": {"multi_instance": True, "multi_version": False},
+                "namespace": "syn-baz",
+            },
+        }
     }
 
     with pytest.raises(click.ClickException) as e:
@@ -97,12 +117,23 @@ def test_verify_component_aliases_explicit_no_multiversion_in_alias_exception(co
 def test_verify_component_multiversion_exception(config):
     alias_data = {"baz": "bar"}
     config.register_component_aliases(alias_data)
-    params = {
-        "components": {
-            "bar": {"url": "foo", "version": "v1.0.0"},
-            "baz": {"version": "v1.1.0"},
-        },
-        "bar": {"_metadata": {"multi_instance": True}},
+    params = make_inventory(
+        {
+            "components": {
+                "bar": {"url": "foo", "version": "v1.0.0"},
+                "baz": {"version": "v1.1.0"},
+            },
+            "bar": {"_metadata": {"multi_instance": True}},
+        }
+    )
+    # Simulate merged target params for `baz` -> parameters key is `bar`
+    params["baz"] = {
+        "parameters": {
+            "bar": {
+                "_metadata": {"multi_instance": True},
+                "namespace": "syn-baz",
+            },
+        }
     }
 
     with pytest.raises(click.ClickException) as e:
@@ -117,16 +148,26 @@ def test_verify_component_multiversion_exception(config):
 def test_verify_component_multiversion(config):
     alias_data = {"baz": "bar"}
     config.register_component_aliases(alias_data)
-    params = {
-        "components": {
-            "bar": {"url": "foo", "version": "v1.0.0"},
-            "baz": {"version": "v1.1.0"},
-        },
-        "bar": {
-            "_metadata": {"multi_instance": True, "multi_version": True},
-            "namespace": "syn-bar",
-        },
-        "baz": {"_metadata": {"multi_version": True}, "namespace": "syn-baz"},
+    params = make_inventory(
+        {
+            "components": {
+                "bar": {"url": "foo", "version": "v1.0.0"},
+                "baz": {"version": "v1.1.0"},
+            },
+            "bar": {
+                "_metadata": {"multi_instance": True, "multi_version": True},
+                "namespace": "syn-bar",
+            },
+        }
+    )
+    # Simulate merged target params for `baz` -> parameters key is `bar`
+    params["baz"] = {
+        "parameters": {
+            "bar": {
+                "_metadata": {"multi_instance": True, "multi_version": True},
+                "namespace": "syn-baz",
+            },
+        }
     }
 
     config.verify_component_aliases(params)
@@ -135,7 +176,9 @@ def test_verify_component_multiversion(config):
 def test_verify_component_aliases_metadata(config):
     alias_data = {"baz": "bar"}
     config.register_component_aliases(alias_data)
-    params = {"bar": {"_metadata": {"multi_instance": True}, "namespace": "syn-bar"}}
+    params = make_inventory(
+        {"bar": {"_metadata": {"multi_instance": True}, "namespace": "syn-bar"}}
+    )
 
     config.verify_component_aliases(params)
 
@@ -145,7 +188,7 @@ def test_verify_component_aliases_metadata(config):
 def test_verify_toplevel_component_aliases_exception(config):
     alias_data = {"baz": "bar"}
     config.register_component_aliases(alias_data)
-    params = {"bar": {"multi_instance": True, "namespace": "syn-bar"}}
+    params = make_inventory({"bar": {"multi_instance": True, "namespace": "syn-bar"}})
 
     with pytest.raises(click.ClickException) as e:
         config.verify_component_aliases(params)
@@ -158,7 +201,7 @@ def test_verify_toplevel_component_aliases_exception(config):
 def test_verify_component_aliases_error(config):
     alias_data = {"baz": "bar"}
     config.register_component_aliases(alias_data)
-    params = {"bar": {"namespace": "syn-bar"}}
+    params = make_inventory({"bar": {"namespace": "syn-bar"}})
 
     with pytest.raises(click.ClickException):
         config.verify_component_aliases(params)
@@ -167,7 +210,9 @@ def test_verify_component_aliases_error(config):
 def test_verify_component_aliases_explicit_no_instance_error(config):
     alias_data = {"baz": "bar"}
     config.register_component_aliases(alias_data)
-    params = {"bar": {"_metadata": {"multi_instance": False}, "namespace": "syn-bar"}}
+    params = make_inventory(
+        {"bar": {"_metadata": {"multi_instance": False}, "namespace": "syn-bar"}}
+    )
 
     with pytest.raises(click.ClickException):
         config.verify_component_aliases(params)