Disambiguate Helm dependencies considering the explictly provided one…

…s. (pantsbuild#15096)

Closes pantsbuild#15094 and closes pantsbuild#15095, which were discovered when working on a PoC for producing Helm lockfiles. 

Removing the need to configure Helm classic repositories leads to a more general solution when fetching 3rd party artifacts and makes it easier to pupulate into the chart's metadata any dependency that may not been listed in `Chart.yaml`, but specified in the `dependencies` field of a `helm_chart`.

src/python/pants/backend/helm/dependency_inference/chart.py

@@ -1,28 +1,48 @@
 # Copyright 2022 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
+from __future__ import annotations
+
 import logging
+from typing import Iterable
 
 from pants.backend.helm.resolve import artifacts
-from pants.backend.helm.resolve.artifacts import (
-    FirstPartyArtifactMapping,
-    ThirdPartyArtifactMapping,
-)
+from pants.backend.helm.resolve.artifacts import ThirdPartyHelmArtifactMapping
 from pants.backend.helm.subsystems.helm import HelmSubsystem
-from pants.backend.helm.target_types import HelmChartMetaSourceField
+from pants.backend.helm.target_types import (
+    AllHelmChartTargets,
+    HelmChartDependenciesField,
+    HelmChartMetaSourceField,
+    HelmChartTarget,
+)
 from pants.backend.helm.util_rules.chart_metadata import HelmChartDependency, HelmChartMetadata
 from pants.engine.addresses import Address
-from pants.engine.internals.selectors import Get
+from pants.engine.internals.selectors import Get, MultiGet
 from pants.engine.rules import collect_rules, rule
-from pants.engine.target import InferDependenciesRequest, InferredDependencies
+from pants.engine.target import (
+    DependenciesRequest,
+    ExplicitlyProvidedDependencies,
+    InferDependenciesRequest,
+    InferredDependencies,
+    WrappedTarget,
+)
 from pants.engine.unions import UnionRule
+from pants.util.frozendict import FrozenDict
 from pants.util.logging import LogLevel
 from pants.util.ordered_set import OrderedSet
-from pants.util.strutil import pluralize
+from pants.util.strutil import bullet_list, pluralize
 
 logger = logging.getLogger(__name__)
 
 
+class DuplicateHelmChartNamesFound(Exception):
+    def __init__(self, duplicates: Iterable[tuple[str, Address]]) -> None:
+        super().__init__(
+            f"Found more than one `{HelmChartTarget.alias}` target using the same chart name:\n\n"
+            f"{bullet_list([f'{addr} -> {name}' for name, addr in duplicates])}"
+        )
+
+
 class UnknownHelmChartDependency(Exception):
     def __init__(self, address: Address, dependency: HelmChartDependency) -> None:
         super().__init__(
@@ -31,35 +51,97 @@ def __init__(self, address: Address, dependency: HelmChartDependency) -> None:
         )
 
 
+class FirstPartyHelmChartMapping(FrozenDict[str, Address]):
+    pass
+
+
+@rule
+async def first_party_helm_chart_mapping(
+    all_helm_chart_tgts: AllHelmChartTargets,
+) -> FirstPartyHelmChartMapping:
+    charts_metadata = await MultiGet(
+        Get(HelmChartMetadata, HelmChartMetaSourceField, tgt[HelmChartMetaSourceField])
+        for tgt in all_helm_chart_tgts
+    )
+
+    name_addr_mapping: dict[str, Address] = {}
+    duplicate_chart_names: OrderedSet[tuple[str, Address]] = OrderedSet()
+
+    for meta, tgt in zip(charts_metadata, all_helm_chart_tgts):
+        if meta.name in name_addr_mapping:
+            duplicate_chart_names.add((meta.name, name_addr_mapping[meta.name]))
+            duplicate_chart_names.add((meta.name, tgt.address))
+            continue
+
+        name_addr_mapping[meta.name] = tgt.address
+
+    if duplicate_chart_names:
+        raise DuplicateHelmChartNamesFound(duplicate_chart_names)
+
+    return FirstPartyHelmChartMapping(name_addr_mapping)
+
+
 class InferHelmChartDependenciesRequest(InferDependenciesRequest):
     infer_from = HelmChartMetaSourceField
 
 
 @rule(desc="Inferring Helm chart dependencies", level=LogLevel.DEBUG)
 async def infer_chart_dependencies_via_metadata(
     request: InferHelmChartDependenciesRequest,
-    first_party_mapping: FirstPartyArtifactMapping,
-    third_party_mapping: ThirdPartyArtifactMapping,
+    first_party_mapping: FirstPartyHelmChartMapping,
+    third_party_mapping: ThirdPartyHelmArtifactMapping,
     subsystem: HelmSubsystem,
 ) -> InferredDependencies:
+    original_addr = request.sources_field.address
+    wrapped_tgt = await Get(WrappedTarget, Address, original_addr)
+    tgt = wrapped_tgt.target
+
     # Parse Chart.yaml for explicitly set dependencies.
-    metadata = await Get(HelmChartMetadata, HelmChartMetaSourceField, request.sources_field)
+    explicitly_provided_deps, metadata = await MultiGet(
+        Get(ExplicitlyProvidedDependencies, DependenciesRequest(tgt[HelmChartDependenciesField])),
+        Get(HelmChartMetadata, HelmChartMetaSourceField, request.sources_field),
+    )
+
+    remotes = subsystem.remotes()
+
+    def resolve_dependency_url(dependency: HelmChartDependency) -> str | None:
+        if not dependency.repository:
+            registry = remotes.default_registry
+            if registry:
+                return f"{registry.address}/{dependency.name}"
+            return None
+        else:
+            return f"{dependency.repository}/{dependency.name}"
 
     # Associate dependencies in Chart.yaml with addresses.
     dependencies: OrderedSet[Address] = OrderedSet()
     for chart_dep in metadata.dependencies:
-        # Check if this is a third party dependency declared as `helm_artifact`.
-        artifact_addr = third_party_mapping.get(chart_dep.remote_spec(subsystem.remotes()))
-        if artifact_addr:
-            dependencies.add(artifact_addr)
-            continue
+        candidate_addrs = []
+
+        first_party_dep = first_party_mapping.get(chart_dep.name)
+        if first_party_dep:
+            candidate_addrs.append(first_party_dep)
+
+        dependency_url = resolve_dependency_url(chart_dep)
+        third_party_dep = third_party_mapping.get(dependency_url) if dependency_url else None
+        if third_party_dep:
+            candidate_addrs.append(third_party_dep)
 
-        # Treat the dependency as a first party one.
-        first_party_addr = first_party_mapping.get(chart_dep.name)
-        if not first_party_addr:
-            raise UnknownHelmChartDependency(request.sources_field.address, chart_dep)
+        if not candidate_addrs:
+            raise UnknownHelmChartDependency(original_addr, chart_dep)
+
+        matches = frozenset(candidate_addrs).difference(explicitly_provided_deps.includes)
+
+        explicitly_provided_deps.maybe_warn_of_ambiguous_dependency_inference(
+            matches,
+            original_addr,
+            context=f"The Helm chart {original_addr} declares `{chart_dep.name}` as dependency",
+            import_reference="helm dependency",
+        )
 
-        dependencies.add(first_party_addr)
+        maybe_disambiguated = explicitly_provided_deps.disambiguated(matches)
+        if maybe_disambiguated:
+            dependencies.add(maybe_disambiguated)
 
     logger.debug(
         f"Inferred {pluralize(len(dependencies), 'dependency')} for target at address: {request.sources_field.address}"

src/python/pants/backend/helm/dependency_inference/chart_test.py

@@ -5,7 +5,10 @@
 
 import pytest
 
-from pants.backend.helm.dependency_inference.chart import InferHelmChartDependenciesRequest
+from pants.backend.helm.dependency_inference.chart import (
+    FirstPartyHelmChartMapping,
+    InferHelmChartDependenciesRequest,
+)
 from pants.backend.helm.dependency_inference.chart import rules as chart_infer_rules
 from pants.backend.helm.resolve import artifacts
 from pants.backend.helm.target_types import (
@@ -20,6 +23,7 @@
 from pants.engine.rules import QueryRule
 from pants.engine.target import InferredDependencies
 from pants.testutil.rule_runner import RuleRunner
+from pants.util.strutil import bullet_list
 
 
 @pytest.fixture
@@ -31,24 +35,90 @@ def rule_runner() -> RuleRunner:
             *chart.rules(),
             *chart_infer_rules(),
             *target_types_rules(),
+            QueryRule(FirstPartyHelmChartMapping, ()),
             QueryRule(InferredDependencies, (InferHelmChartDependenciesRequest,)),
         ],
     )
 
 
+def test_build_first_party_mapping(rule_runner: RuleRunner) -> None:
+    rule_runner.write_files(
+        {
+            "src/BUILD": "helm_chart(name='foo')",
+            "src/Chart.yaml": dedent(
+                """\
+                apiVersion: v2
+                name: chart-name
+                version: 0.1.0
+                """
+            ),
+        }
+    )
+
+    tgt = rule_runner.get_target(Address("src", target_name="foo"))
+    mapping = rule_runner.request(FirstPartyHelmChartMapping, [])
+    assert mapping["chart-name"] == tgt.address
+
+
+def test_duplicate_first_party_mappings(rule_runner: RuleRunner) -> None:
+    rule_runner.write_files(
+        {
+            "src/foo/BUILD": "helm_chart()",
+            "src/foo/Chart.yaml": dedent(
+                """\
+                apiVersion: v2
+                name: chart-name
+                version: 0.1.0
+                """
+            ),
+            "src/bar/BUILD": "helm_chart()",
+            "src/bar/Chart.yaml": dedent(
+                """\
+                apiVersion: v2
+                name: chart-name
+                version: 0.1.0
+                """
+            ),
+        }
+    )
+
+    expected_err_msg = (
+        "Found more than one `helm_chart` target using the same chart name:\n\n"
+        f"{bullet_list(['src/bar:bar -> chart-name', 'src/foo:foo -> chart-name'])}"
+    )
+
+    with pytest.raises(ExecutionError) as err_info:
+        rule_runner.request(FirstPartyHelmChartMapping, [])
+
+    assert expected_err_msg in err_info.value.args[0]
+
+
 def test_infer_chart_dependencies(rule_runner: RuleRunner) -> None:
     rule_runner.write_files(
         {
             "3rdparty/helm/jetstack/BUILD": dedent(
                 """\
                 helm_artifact(
                   name="cert-manager",
-                  repository="@jetstack",
+                  repository="https://charts.jetstack.io",
                   artifact="cert-manager",
                   version="v0.7.0"
                 )
                 """
             ),
+            "src/foo/BUILD": """helm_chart(dependencies=["//src/quxx"])""",
+            "src/foo/Chart.yaml": dedent(
+                """\
+                apiVersion: v2
+                name: foo
+                version: 0.1.0
+                dependencies:
+                - name: cert-manager
+                  repository: "https://charts.jetstack.io"
+                - name: bar
+                - name: quxx
+                """
+            ),
             "src/bar/BUILD": """helm_chart()""",
             "src/bar/Chart.yaml": dedent(
                 """\
@@ -57,27 +127,21 @@ def test_infer_chart_dependencies(rule_runner: RuleRunner) -> None:
                 version: 0.1.0
                 """
             ),
-            "src/foo/BUILD": """helm_chart()""",
-            "src/foo/Chart.yaml": dedent(
+            "src/quxx/BUILD": """helm_chart()""",
+            "src/quxx/Chart.yaml": dedent(
                 """\
                 apiVersion: v2
-                name: foo
+                name: quxx
                 version: 0.1.0
-                dependencies:
-                - name: cert-manager
-                  repository: "@jetstack"
-                - name: bar
                 """
             ),
         }
     )
 
     source_root_patterns = ("/src/*",)
-    repositories_opts = """{"jetstack": {"address": "https://charts.jetstack.io"}}"""
     rule_runner.set_options(
         [
             f"--source-root-patterns={repr(source_root_patterns)}",
-            f"--helm-classic-repositories={repositories_opts}",
         ]
     )
 
@@ -91,6 +155,47 @@ def test_infer_chart_dependencies(rule_runner: RuleRunner) -> None:
     }
 
 
+def test_disambiguate_chart_dependencies(rule_runner: RuleRunner) -> None:
+    rule_runner.write_files(
+        {
+            "3rdparty/bar/BUILD": dedent(
+                """\
+                helm_artifact(artifact="bar", version="0.1.0", registry="oci://example.com/charts")
+                """
+            ),
+            "src/foo/BUILD": """helm_chart(dependencies=["!//3rdparty/bar"])""",
+            "src/foo/Chart.yaml": dedent(
+                """\
+                apiVersion: v2
+                name: foo
+                version: 0.1.0
+                dependencies:
+                - name: bar
+                """
+            ),
+            "src/bar/BUILD": """helm_chart()""",
+            "src/bar/Chart.yaml": dedent(
+                """\
+                apiVersion: v2
+                name: bar
+                version: 0.1.0
+                """
+            ),
+        }
+    )
+
+    source_root_patterns = ("/src/*",)
+    rule_runner.set_options([f"--source-root-patterns={repr(source_root_patterns)}"])
+
+    tgt = rule_runner.get_target(Address("src/foo", target_name="foo"))
+    inferred_deps = rule_runner.request(
+        InferredDependencies, [InferHelmChartDependenciesRequest(tgt[HelmChartMetaSourceField])]
+    )
+    assert set(inferred_deps.dependencies) == {
+        Address("src/bar", target_name="bar"),
+    }
+
+
 def test_raise_error_when_unknown_dependency_is_found(rule_runner: RuleRunner) -> None:
     rule_runner.write_files(
         {