Raise an error when a component uses another component's name as library prefix

Until now, Commodore's component library name validation only checked
that each library is prefixed with the component's name. This isn't
sufficient in some cases: for example if we have components `foo` and
`foo-operator`, component foo was allowed to deploy a library
`foo-operator.libsonnet` which could cause non-deterministic symlinking
if component `foo-operator` also deployed the library
`foo-operator.libsonnet`.

This commit introduces an additional check in the component library name
validation which raises an error if a component deploys a library which
is prefixed with another component's name.

Author: simu

commodore/component/compile.py

@@ -174,7 +174,7 @@ def _setup_component(
 
     # Validate component libraries
     for lib in component.lib_files:
-        validate_component_library_name(component.name, lib)
+        validate_component_library_name(config.get_components(), component.name, lib)
 
     return component
 

commodore/dependency_mgmt/__init__.py

@@ -19,7 +19,7 @@
 from .version_parsing import _read_components, _read_packages, DependencySpec
 
 
-def create_component_symlinks(cfg, component: Component):
+def create_component_symlinks(cfg: Config, component: Component):
     """
     Create symlinks in the inventory subdirectory.
 
@@ -38,7 +38,7 @@ def create_component_symlinks(cfg, component: Component):
         if cfg.debug:
             click.echo(f"     > installing template library: {file}")
         relsymlink(
-            validate_component_library_name(component.name, file),
+            validate_component_library_name(cfg.get_components(), component.name, file),
             cfg.inventory.lib_dir,
         )
 

commodore/dependency_mgmt/component_library.py

@@ -12,7 +12,18 @@
 from .tools import format_component_list
 
 
-def validate_component_library_name(cname: str, lib: Path) -> Path:
+def validate_component_library_name(
+    components: dict[str, Component], cname: str, lib: Path
+) -> Path:
+    # This variable holds the set of component names which are prefixed with `cname` but aren't
+    # cname. We need to prohibit component library names that are prefixed with another component's
+    # name even if they're prefixed with our name to avoid non-deterministic library symlinks when
+    # e.g. components `foo` and `foo-operator` both define `foo-operator.libsonnet`.
+    other_components = set(c for c in components if c != cname and c.startswith(cname))
+    if any(lib.stem.startswith(c) for c in other_components):
+        raise click.ClickException(
+            f"Component '{cname}' defines component library '{lib.name}' which is reserved for another component."
+        )
     if not lib.stem.startswith(cname):
         raise click.ClickException(
             f"Component '{cname}' uses invalid component library name '{lib.name}'. "

docs/modules/ROOT/pages/reference/concepts.adoc

@@ -57,7 +57,9 @@ inventory to install the software.
 Components can optionally provide Jsonnet libraries which can be used by other components.
 To allow Commodore to make component libraries available to other components, they must be placed in directory `lib/`.
 Components must ensure that all Jsonnet libraries which they provide are prefixed with the component name.
+Additionally, components must ensure that their Jsonnet library names don't collide with other component names that are prefixed by the component's name.
 For example, component `foo-bar` can only contain libraries whose file name starts with `foo-bar`, such as `foo-bar.libsonnet`, or `foo-bar-baz.libsonnet`.
+However, note that the library name `foo-bar-baz.libsonnet` would be prohibited if component `foo-bar-baz` also exists.
 Commodore allows components to advertise aliases for their component libraries.
 Component library aliases are described in more detail in the xref:commodore:ROOT:reference/architecture.adoc#_component_template_libraries[architecture documentation].
 

tests/test_dependency_mgmt.py

@@ -483,27 +483,50 @@ def test_register_dangling_aliases(
 
 
 @pytest.mark.parametrize(
-    "libname,expected",
+    "cnames,libname,expected",
     [
-        ("test-component.libsonnet", ""),
-        ("test-component-lib.libsonnet", ""),
+        (["test-component"], "test-component.libsonnet", ""),
+        (["test-component"], "test-component-lib.libsonnet", ""),
         (
+            ["test-component"],
             "lib.libsonnet",
             "Component 'test-component' uses invalid component library name 'lib.libsonnet'. "
             + "Consider using a library alias.",
         ),
+        (
+            ["test-component", "test-component-operator"],
+            "test-component-operator.libsonnet",
+            "Component 'test-component' defines component library "
+            + "'test-component-operator.libsonnet' which is reserved for another component.",
+        ),
     ],
 )
-def test_validate_component_library_name(tmp_path: Path, libname: str, expected: str):
+def test_validate_component_library_name(
+    config: Config, tmp_path: Path, cnames: list[str], libname: str, expected: str
+):
+    deps = setup_components_upstream(tmp_path, cnames, {})
+    for cn, cspec in deps.items():
+        cdep = config.register_dependency_repo(cspec.url)
+        component = Component(
+            cn,
+            work_dir=tmp_path,
+            dependency=cdep,
+        )
+        config.register_component(component)
+
     if expected == "":
         lpath = Path(tmp_path / "lib" / libname)
-        r = dependency_mgmt.validate_component_library_name("test-component", lpath)
+        r = dependency_mgmt.validate_component_library_name(
+            config.get_components(), "test-component", lpath
+        )
         assert lpath == r
 
     else:
         with pytest.raises(click.ClickException) as e:
             dependency_mgmt.validate_component_library_name(
-                "test-component", Path(tmp_path / "lib" / libname)
+                config.get_components(),
+                "test-component",
+                Path(tmp_path / "lib" / libname),
             )
 
         assert expected in str(e.value)