From a3a78355436fcb7eac2200edc731b815056ae92c Mon Sep 17 00:00:00 2001 From: "A. Alonso Dominguez" <2269440+alonsodomin@users.noreply.github.com> Date: Wed, 13 Apr 2022 19:18:08 +0200 Subject: [PATCH] Disambiguate Helm dependencies considering the explictly provided ones. (#15096) Closes #15094 and closes #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`. --- .../helm/dependency_inference/chart.py | 124 ++++++++++++++--- .../helm/dependency_inference/chart_test.py | 127 ++++++++++++++++-- .../pants/backend/helm/resolve/artifacts.py | 119 +++++++++------- .../backend/helm/resolve/artifacts_test.py | 118 +++++----------- .../pants/backend/helm/resolve/fetch.py | 74 ++++++---- .../pants/backend/helm/resolve/fetch_test.py | 20 ++- .../pants/backend/helm/resolve/remotes.py | 58 ++------ .../backend/helm/resolve/remotes_test.py | 24 ++-- .../pants/backend/helm/subsystems/helm.py | 32 +---- src/python/pants/backend/helm/target_types.py | 14 +- .../pants/backend/helm/util_rules/chart.py | 39 ++++-- .../backend/helm/util_rules/chart_metadata.py | 13 -- .../backend/helm/util_rules/chart_test.py | 88 +++++++++++- .../pants/backend/helm/util_rules/tool.py | 106 ++++++--------- .../backend/helm/util_rules/tool_test.py | 29 +--- 15 files changed, 572 insertions(+), 413 deletions(-) diff --git a/src/python/pants/backend/helm/dependency_inference/chart.py b/src/python/pants/backend/helm/dependency_inference/chart.py index 94567540e79..467ad543ff2 100644 --- a/src/python/pants/backend/helm/dependency_inference/chart.py +++ b/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,6 +51,36 @@ 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 @@ -38,28 +88,60 @@ class InferHelmChartDependenciesRequest(InferDependenciesRequest): @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}" diff --git a/src/python/pants/backend/helm/dependency_inference/chart_test.py b/src/python/pants/backend/helm/dependency_inference/chart_test.py index f704ec3deab..2f95564fc94 100644 --- a/src/python/pants/backend/helm/dependency_inference/chart_test.py +++ b/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,11 +35,64 @@ 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( { @@ -43,12 +100,25 @@ def test_infer_chart_dependencies(rule_runner: RuleRunner) -> None: """\ 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( { diff --git a/src/python/pants/backend/helm/resolve/artifacts.py b/src/python/pants/backend/helm/resolve/artifacts.py index 64887ce8f9b..b28b8bfe8ef 100644 --- a/src/python/pants/backend/helm/resolve/artifacts.py +++ b/src/python/pants/backend/helm/resolve/artifacts.py @@ -3,28 +3,23 @@ from __future__ import annotations +from abc import ABC from dataclasses import dataclass from typing import Iterable, cast -from pants.backend.helm.resolve.remotes import HelmRemotes from pants.backend.helm.subsystems.helm import HelmSubsystem from pants.backend.helm.target_types import ( AllHelmArtifactTargets, - AllHelmChartTargets, HelmArtifactFieldSet, HelmArtifactRegistryField, HelmArtifactRepositoryField, - HelmChartMetaSourceField, HelmChartTarget, ) -from pants.backend.helm.util_rules.chart_metadata import HelmChartMetadata from pants.backend.helm.util_rules.chart_metadata import rules as metadata_rules from pants.engine.addresses import Address from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.engine.target import Target from pants.util.frozendict import FrozenDict -from pants.util.memo import memoized_method -from pants.util.ordered_set import OrderedSet from pants.util.strutil import bullet_list @@ -44,22 +39,44 @@ def __init__(self, duplicates: Iterable[tuple[str, Address]]) -> None: ) +class HelmArtifactLocationSpec(ABC): + @property + def spec(self) -> str: + pass + + @property + def is_url(self) -> bool: + return len(self.spec.split("://")) == 2 + + @property + def is_alias(self) -> bool: + return self.spec.startswith("@") + + @dataclass(frozen=True) -class HelmArtifactRegistryLocation: +class HelmArtifactRegistryLocationSpec(HelmArtifactLocationSpec): registry: str repository: str | None + @property + def spec(self) -> str: + return self.registry + @dataclass(frozen=True) -class HelmArtifactClassicRepositoryLocation: +class HelmArtifactClassicRepositoryLocationSpec(HelmArtifactLocationSpec): repository: str + @property + def spec(self) -> str: + return self.repository + @dataclass(frozen=True) class HelmArtifactRequirement: name: str version: str - location: HelmArtifactRegistryLocation | HelmArtifactClassicRepositoryLocation + location: HelmArtifactLocationSpec @dataclass(frozen=True) @@ -78,11 +95,13 @@ def from_field_set(cls, field_set: HelmArtifactFieldSet) -> HelmArtifact: if not registry and not repository: raise MissingHelmArtifactLocation(field_set.address) - registry_location: HelmArtifactRegistryLocation | None = None + registry_location: HelmArtifactRegistryLocationSpec | None = None if registry: - registry_location = HelmArtifactRegistryLocation(registry, repository) + registry_location = HelmArtifactRegistryLocationSpec(registry.rstrip("/"), repository) - location = registry_location or HelmArtifactClassicRepositoryLocation(cast(str, repository)) + location = registry_location or HelmArtifactClassicRepositoryLocationSpec( + cast(str, repository).rstrip("/") + ) req = HelmArtifactRequirement( name=cast(str, field_set.artifact.value), version=cast(str, field_set.version.value), @@ -95,59 +114,57 @@ def from_field_set(cls, field_set: HelmArtifactFieldSet) -> HelmArtifact: def name(self) -> str: return self.requirement.name - @memoized_method - def remote_address(self, remotes: HelmRemotes) -> str: - if isinstance(self.requirement.location, HelmArtifactRegistryLocation): - remote = next(remotes.get(self.requirement.location.registry)) - repo_ref = f"{remote.address}/{self.requirement.location.repository or ''}".rstrip("/") - else: - remote = next(remotes.get(self.requirement.location.repository)) - repo_ref = remote.alias - - return f"{repo_ref}/{self.name}" - + @property + def version(self) -> str: + return self.requirement.version -class FirstPartyArtifactMapping(FrozenDict[str, Address]): - pass +@dataclass(frozen=True) +class ResolvedHelmArtifact(HelmArtifact): + location_url: str -@rule -async def first_party_artifact_mapping( - all_helm_chart_tgts: AllHelmChartTargets, -) -> FirstPartyArtifactMapping: - charts_metadata = await MultiGet( - Get(HelmChartMetadata, HelmChartMetaSourceField, tgt[HelmChartMetaSourceField]) - for tgt in all_helm_chart_tgts - ) + @classmethod + def from_unresolved(cls, artifact: HelmArtifact, *, location_url: str) -> ResolvedHelmArtifact: + return cls( + requirement=artifact.requirement, + address=artifact.address, + location_url=location_url, + ) - name_addr_mapping: dict[str, Address] = {} - duplicate_chart_names: OrderedSet[tuple[str, Address]] = OrderedSet() + @property + def chart_url(self) -> str: + return f"{self.location_url}/{self.name}" - 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 +@rule +def resolved_helm_artifact(artifact: HelmArtifact, subsytem: HelmSubsystem) -> ResolvedHelmArtifact: + remotes = subsytem.remotes() - if duplicate_chart_names: - raise DuplicateHelmChartNamesFound(duplicate_chart_names) + candidate_remotes = list(remotes.get(artifact.requirement.location.spec)) + if candidate_remotes: + loc_url = candidate_remotes[0].address + if isinstance(artifact.requirement.location, HelmArtifactRegistryLocationSpec): + loc_url = f"{loc_url}/{artifact.requirement.location.repository or ''}".rstrip("/") + else: + loc_url = artifact.requirement.location.spec - return FirstPartyArtifactMapping(name_addr_mapping) + return ResolvedHelmArtifact.from_unresolved(artifact, location_url=loc_url) -class ThirdPartyArtifactMapping(FrozenDict[str, Address]): +class ThirdPartyHelmArtifactMapping(FrozenDict[str, Address]): pass @rule -def third_party_artifact_mapping( - all_helm_artifact_tgts: AllHelmArtifactTargets, subsystem: HelmSubsystem -) -> ThirdPartyArtifactMapping: - artifacts = [HelmArtifact.from_target(tgt) for tgt in all_helm_artifact_tgts] - return ThirdPartyArtifactMapping( - {artifact.remote_address(subsystem.remotes()): artifact.address for artifact in artifacts} +async def third_party_helm_artifact_mapping( + all_helm_artifact_tgts: AllHelmArtifactTargets, +) -> ThirdPartyHelmArtifactMapping: + artifacts = await MultiGet( + Get(ResolvedHelmArtifact, HelmArtifact, HelmArtifact.from_target(tgt)) + for tgt in all_helm_artifact_tgts + ) + return ThirdPartyHelmArtifactMapping( + {artifact.chart_url: artifact.address for artifact in artifacts} ) diff --git a/src/python/pants/backend/helm/resolve/artifacts_test.py b/src/python/pants/backend/helm/resolve/artifacts_test.py index 0c430cb9c0d..ad933c58065 100644 --- a/src/python/pants/backend/helm/resolve/artifacts_test.py +++ b/src/python/pants/backend/helm/resolve/artifacts_test.py @@ -8,12 +8,11 @@ import pytest from pants.backend.helm.resolve.artifacts import ( - FirstPartyArtifactMapping, HelmArtifact, - HelmArtifactClassicRepositoryLocation, - HelmArtifactRegistryLocation, + HelmArtifactClassicRepositoryLocationSpec, + HelmArtifactRegistryLocationSpec, HelmArtifactRequirement, - ThirdPartyArtifactMapping, + ThirdPartyHelmArtifactMapping, ) from pants.backend.helm.resolve.artifacts import rules as artifacts_rules from pants.backend.helm.target_types import ( @@ -22,16 +21,9 @@ HelmChartTarget, ) from pants.backend.helm.target_types import rules as target_types_rules -from pants.backend.helm.testutil import ( - HELM_TEMPLATE_HELPERS_FILE, - HELM_VALUES_FILE, - K8S_SERVICE_FILE, -) from pants.engine.addresses import Address -from pants.engine.internals.scheduler import ExecutionError from pants.engine.rules import QueryRule from pants.testutil.rule_runner import RuleRunner -from pants.util.strutil import bullet_list @pytest.fixture @@ -42,73 +34,11 @@ def rule_runner() -> RuleRunner: *artifacts_rules(), *target_types_rules(), QueryRule(AllHelmArtifactTargets, ()), - QueryRule(FirstPartyArtifactMapping, ()), - QueryRule(ThirdPartyArtifactMapping, ()), + QueryRule(ThirdPartyHelmArtifactMapping, ()), ], ) -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 - """ - ), - "src/values.yaml": HELM_VALUES_FILE, - "src/templates/_helpers.tpl": HELM_TEMPLATE_HELPERS_FILE, - "src/templates/service.yaml": K8S_SERVICE_FILE, - } - ) - - tgt = rule_runner.get_target(Address("src", target_name="foo")) - mapping = rule_runner.request(FirstPartyArtifactMapping, []) - assert mapping["chart-name"] == tgt.address - - -def test_duplicate_first_party_artifact_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/foo/values.yaml": HELM_VALUES_FILE, - "src/foo/templates/_helpers.tpl": HELM_TEMPLATE_HELPERS_FILE, - "src/foo/templates/service.yaml": K8S_SERVICE_FILE, - "src/bar/BUILD": "helm_chart()", - "src/bar/Chart.yaml": dedent( - """\ - apiVersion: v2 - name: chart-name - version: 0.1.0 - """ - ), - "src/bar/values.yaml": HELM_VALUES_FILE, - "src/bar/templates/_helpers.tpl": HELM_TEMPLATE_HELPERS_FILE, - "src/bar/templates/service.yaml": K8S_SERVICE_FILE, - } - ) - - 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(FirstPartyArtifactMapping, []) - - assert expected_err_msg in err_info.value.args[0] - - def test_build_third_party_mapping(rule_runner: RuleRunner) -> None: rule_runner.write_files( { @@ -116,17 +46,24 @@ def test_build_third_party_mapping(rule_runner: RuleRunner) -> None: """\ helm_artifact( name="foo", - registry="oci://www.example.com", + registry="oci://www.example.com/", repository="charts", artifact="foo", version="0.1.0", ) helm_artifact( name="bar", - repository="@example", + repository="https://www.example.com/charts", artifact="bar", version="0.1.0", ) + helm_artifact( + name="quux", + registry="@quux", + repository="quux-charts", + artifact="quux", + version="0.1.0", + ) """ ) } @@ -136,7 +73,7 @@ def test_build_third_party_mapping(rule_runner: RuleRunner) -> None: requirement=HelmArtifactRequirement( name="foo", version="0.1.0", - location=HelmArtifactRegistryLocation( + location=HelmArtifactRegistryLocationSpec( registry="oci://www.example.com", repository="charts" ), ), @@ -146,21 +83,36 @@ def test_build_third_party_mapping(rule_runner: RuleRunner) -> None: requirement=HelmArtifactRequirement( name="bar", version="0.1.0", - location=HelmArtifactClassicRepositoryLocation("@example"), + location=HelmArtifactClassicRepositoryLocationSpec("https://www.example.com/charts"), ), address=Address("3rdparty/helm/example", target_name="bar"), ) + expected_quux = HelmArtifact( + requirement=HelmArtifactRequirement( + name="quux", + version="0.1.0", + location=HelmArtifactRegistryLocationSpec(registry="@quux", repository="quux-charts"), + ), + address=Address("3rdparty/helm/example", target_name="quux"), + ) - repositories_opts = """{"example": {"address": "https://www.example.com/charts"}}""" - rule_runner.set_options([f"--helm-classic-repositories={repositories_opts}"]) + registries_opts = """{"quux": {"address": "oci://www.example.com"}}""" + rule_runner.set_options( + [ + f"--helm-registries={registries_opts}", + ] + ) tgts = rule_runner.request(AllHelmArtifactTargets, []) artifacts = [HelmArtifact.from_target(tgt) for tgt in tgts] - assert len(artifacts) == 2 + assert len(artifacts) == 3 assert expected_foo in artifacts assert expected_bar in artifacts + assert expected_quux in artifacts - mapping = rule_runner.request(ThirdPartyArtifactMapping, []) + mapping = rule_runner.request(ThirdPartyHelmArtifactMapping, []) + assert len(mapping) == 3 assert mapping["oci://www.example.com/charts/foo"] == expected_foo.address - assert mapping["example/bar"] == expected_bar.address + assert mapping["https://www.example.com/charts/bar"] == expected_bar.address + assert mapping["oci://www.example.com/quux-charts/quux"] == expected_quux.address diff --git a/src/python/pants/backend/helm/resolve/fetch.py b/src/python/pants/backend/helm/resolve/fetch.py index 943b0ccc26e..034bd4a6491 100644 --- a/src/python/pants/backend/helm/resolve/fetch.py +++ b/src/python/pants/backend/helm/resolve/fetch.py @@ -4,18 +4,26 @@ from __future__ import annotations import logging +import os from dataclasses import dataclass from typing import Iterable from pants.backend.helm.resolve import artifacts -from pants.backend.helm.resolve.artifacts import HelmArtifact -from pants.backend.helm.subsystems.helm import HelmSubsystem +from pants.backend.helm.resolve.artifacts import HelmArtifact, ResolvedHelmArtifact from pants.backend.helm.target_types import HelmArtifactFieldSet from pants.backend.helm.util_rules.tool import HelmProcess from pants.engine.addresses import Address from pants.engine.collection import Collection from pants.engine.engine_aware import EngineAwareParameter -from pants.engine.fs import CreateDigest, Digest, Directory, RemovePrefix, Snapshot +from pants.engine.fs import ( + CreateDigest, + Digest, + DigestSubset, + Directory, + PathGlobs, + RemovePrefix, + Snapshot, +) from pants.engine.process import ProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.engine.target import Target @@ -28,7 +36,7 @@ @dataclass(frozen=True) class FetchedHelmArtifact: - artifact: HelmArtifact + artifact: ResolvedHelmArtifact snapshot: Snapshot @property @@ -70,44 +78,56 @@ def debug_hint(self) -> str | None: @rule(desc="Fetch third party Helm Chart artifacts", level=LogLevel.DEBUG) -async def fetch_helm_artifacts( - request: FetchHelmArfifactsRequest, subsystem: HelmSubsystem -) -> FetchedHelmArtifacts: - remotes = subsystem.remotes() - +async def fetch_helm_artifacts(request: FetchHelmArfifactsRequest) -> FetchedHelmArtifacts: download_prefix = "__downloads" empty_download_digest = await Get(Digest, CreateDigest([Directory(download_prefix)])) - artifacts = [HelmArtifact.from_field_set(fs) for fs in request.field_sets] + artifacts = await MultiGet( + Get(ResolvedHelmArtifact, HelmArtifact, HelmArtifact.from_field_set(field_set)) + for field_set in request.field_sets + ) + + def create_fetch_process(artifact: ResolvedHelmArtifact) -> HelmProcess: + return HelmProcess( + argv=[ + "pull", + artifact.name, + "--repo", + artifact.location_url, + "--version", + artifact.version, + "--destination", + download_prefix, + "--untar", + ], + input_digest=empty_download_digest, + description=f"Pulling Helm Chart '{artifact.name}' with version {artifact.version}", + output_directories=(download_prefix,), + ) + download_results = await MultiGet( Get( ProcessResult, - HelmProcess( - argv=[ - "pull", - artifact.remote_address(remotes), - "--version", - artifact.requirement.version, - "--destination", - download_prefix, - "--untar", - ], - input_digest=empty_download_digest, - description=f"Pulling Helm Chart '{artifact.requirement.name}' with version {artifact.requirement.version}", - output_directories=(download_prefix,), - ), + HelmProcess, + create_fetch_process(artifact), ) for artifact in artifacts ) - stripped_artifact_snapshots = await MultiGet( - Get(Snapshot, RemovePrefix(result.output_digest, download_prefix)) + stripped_artifact_digests = await MultiGet( + Get(Digest, RemovePrefix(result.output_digest, download_prefix)) for result in download_results ) + # Avoid capturing the tarball that has been downloaded by Helm during the pull. + artifact_snapshots = await MultiGet( + Get(Snapshot, DigestSubset(digest, PathGlobs([os.path.join(artifact.name, "**")]))) + for artifact, digest in zip(artifacts, stripped_artifact_digests) + ) + fetched_artifacts = [ FetchedHelmArtifact(artifact=artifact, snapshot=snapshot) - for artifact, snapshot in zip(artifacts, stripped_artifact_snapshots) + for artifact, snapshot in zip(artifacts, artifact_snapshots) ] logger.debug( f"Fetched {pluralize(len(fetched_artifacts), 'Helm artifact')} corresponding with:\n" diff --git a/src/python/pants/backend/helm/resolve/fetch_test.py b/src/python/pants/backend/helm/resolve/fetch_test.py index 48bdaec05b7..58f577bf179 100644 --- a/src/python/pants/backend/helm/resolve/fetch_test.py +++ b/src/python/pants/backend/helm/resolve/fetch_test.py @@ -8,7 +8,7 @@ import pytest from pants.backend.helm.resolve import fetch -from pants.backend.helm.resolve.artifacts import HelmArtifact +from pants.backend.helm.resolve.artifacts import HelmArtifact, ResolvedHelmArtifact from pants.backend.helm.resolve.fetch import FetchedHelmArtifacts, FetchHelmArfifactsRequest from pants.backend.helm.target_types import AllHelmArtifactTargets, HelmArtifactTarget from pants.backend.helm.target_types import rules as target_types_rules @@ -31,6 +31,7 @@ def rule_runner() -> RuleRunner: *process.rules(), *target_types_rules(), QueryRule(AllHelmArtifactTargets, ()), + QueryRule(ResolvedHelmArtifact, (HelmArtifact,)), QueryRule(FetchedHelmArtifacts, (FetchHelmArfifactsRequest,)), ], ) @@ -43,14 +44,14 @@ def test_download_artifacts(rule_runner: RuleRunner) -> None: """\ helm_artifact( name="cert-manager", - repository="@jetstack", + repository="https://charts.jetstack.io/", artifact="cert-manager", - version="v0.7.0" + version="v1.7.1" ) helm_artifact( name="prometheus-stack", - repository="@prometheus", + repository="https://prometheus-community.github.io/helm-charts", artifact="kube-prometheus-stack", version="^27.2.0" ) @@ -59,12 +60,6 @@ def test_download_artifacts(rule_runner: RuleRunner) -> None: } ) - repositories_opts = { - "jetstack": {"address": "https://charts.jetstack.io"}, - "prometheus": {"address": "https://prometheus-community.github.io/helm-charts"}, - } - rule_runner.set_options([f"--helm-classic-repositories={repr(repositories_opts)}"]) - targets = rule_runner.request(AllHelmArtifactTargets, []) fetched_artifacts = rule_runner.request( FetchedHelmArtifacts, @@ -75,7 +70,10 @@ def test_download_artifacts(rule_runner: RuleRunner) -> None: ], ) - expected_artifacts = [HelmArtifact.from_target(tgt) for tgt in targets] + expected_artifacts = [ + rule_runner.request(ResolvedHelmArtifact, [HelmArtifact.from_target(tgt)]) + for tgt in targets + ] assert len(fetched_artifacts) == len(expected_artifacts) for fetched, expected in zip(fetched_artifacts, expected_artifacts): diff --git a/src/python/pants/backend/helm/resolve/remotes.py b/src/python/pants/backend/helm/resolve/remotes.py index 4b9fb3981f8..5bbdb053c2c 100644 --- a/src/python/pants/backend/helm/resolve/remotes.py +++ b/src/python/pants/backend/helm/resolve/remotes.py @@ -3,7 +3,6 @@ from __future__ import annotations -from abc import ABC from dataclasses import dataclass from typing import Any, Iterator, cast @@ -24,32 +23,15 @@ def __init__(self, alias: str, address: str) -> None: ) -class InvalidHelmClassicRepositoryAddress(ValueError): - def __init__(self, alias: str, address: str) -> None: - super().__init__( - f"The classic repository '{alias}' can not have an OCI address URL " - f"(using protocol '{OCI_REGISTRY_PROTOCOL}'). The given address was instead: {address}" - ) - - class HelmRemoteAliasNotFoundError(ValueError): def __init__(self, alias: str) -> None: super().__init__(f"There is no Helm remote configured with alias: {alias}") @dataclass(frozen=True) -class HelmRemote(ABC): +class HelmRegistry: address: str alias: str = "" - - def register(self, remotes: dict[str, HelmRemote]) -> None: - remotes[self.address] = self - if self.alias: - remotes[f"@{self.alias}"] = self - - -@dataclass(frozen=True) -class HelmRegistry(HelmRemote): default: bool = False @classmethod @@ -64,30 +46,22 @@ def __post_init__(self) -> None: if not self.address.startswith(OCI_REGISTRY_PROTOCOL): raise InvalidHelmRegistryAddress(self.alias, self.address) - -@dataclass(frozen=True) -class HelmClassicRepository(HelmRemote): - @classmethod - def from_dict(cls, alias: str, d: dict[str, Any]) -> HelmClassicRepository: - return cls(alias=alias, address=cast(str, d["address"]).rstrip("/")) - - def __post_init__(self) -> None: - if self.address.startswith(OCI_REGISTRY_PROTOCOL): - raise InvalidHelmClassicRepositoryAddress(self.alias, self.address) + def register(self, remotes: dict[str, HelmRegistry]) -> None: + remotes[self.address] = self + if self.alias: + remotes[f"@{self.alias}"] = self @dataclass(frozen=True) class HelmRemotes: default: tuple[HelmRegistry, ...] - all: FrozenDict[str, HelmRemote] + all: FrozenDict[str, HelmRegistry] @classmethod - def from_dicts(cls, d_regs: dict[str, Any], d_repos: dict[str, Any]) -> HelmRemotes: - remotes: dict[str, HelmRemote] = {} + def from_dict(cls, d_regs: dict[str, Any]) -> HelmRemotes: + remotes: dict[str, HelmRegistry] = {} for alias, opts in d_regs.items(): HelmRegistry.from_dict(alias, opts).register(remotes) - for alias, opts in d_repos.items(): - HelmClassicRepository.from_dict(alias, opts).register(remotes) return cls( default=tuple( sorted( @@ -98,7 +72,7 @@ def from_dicts(cls, d_regs: dict[str, Any], d_repos: dict[str, Any]) -> HelmRemo all=FrozenDict(remotes), ) - def get(self, *aliases_or_addresses: str) -> Iterator[HelmRemote]: + def get(self, *aliases_or_addresses: str) -> Iterator[HelmRegistry]: for alias_or_address in aliases_or_addresses: if alias_or_address in self.all: yield self.all[alias_or_address] @@ -108,19 +82,15 @@ def get(self, *aliases_or_addresses: str) -> Iterator[HelmRemote]: yield from self.default elif alias_or_address.startswith(OCI_REGISTRY_PROTOCOL): yield HelmRegistry(address=alias_or_address) - else: - yield HelmClassicRepository(address=alias_or_address) @memoized_method - def classic_repositories(self) -> tuple[HelmClassicRepository, ...]: - deduped_repos = {r for _, r in self.all.items() if isinstance(r, HelmClassicRepository)} - return tuple(deduped_repos) + def registries(self) -> tuple[HelmRegistry, ...]: + deduped_regs = {r for _, r in self.all.items() if isinstance(r, HelmRegistry)} + return tuple(deduped_regs) @property def default_registry(self) -> HelmRegistry | None: remote = self.all.get("default") - if remote: - return cast(HelmRegistry, remote) if not remote and self.default: - return self.default[0] - return None + remote = self.default[0] + return remote diff --git a/src/python/pants/backend/helm/resolve/remotes_test.py b/src/python/pants/backend/helm/resolve/remotes_test.py index 4ce90e97f58..554c4a5cd53 100644 --- a/src/python/pants/backend/helm/resolve/remotes_test.py +++ b/src/python/pants/backend/helm/resolve/remotes_test.py @@ -1,13 +1,11 @@ # Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -from typing import cast import pytest from pants.backend.helm.resolve.remotes import ( ALL_DEFAULT_HELM_REGISTRIES, - HelmClassicRepository, HelmRegistry, HelmRemoteAliasNotFoundError, HelmRemotes, @@ -15,18 +13,19 @@ def test_helm_remotes() -> None: - remotes = HelmRemotes.from_dicts( - {"reg1": {"address": "oci://www.example.com"}}, - {"repo1": {"address": "https://www.example.com"}}, + remotes = HelmRemotes.from_dict( + { + "reg1": {"address": "oci://www.example.com"}, + "reg2": {"address": "oci://www.example.com/subfolder"}, + }, ) assert remotes.default == () assert list(remotes.get()) == [] assert len(list(remotes.get("@reg1"))) == 1 - assert len(list(remotes.get("@repo1"))) == 1 - assert len(list(remotes.get("@reg1", "@repo1"))) == 2 + assert len(list(remotes.get("@reg1", "@reg2"))) == 2 assert next(remotes.get("@reg1")).address == "oci://www.example.com" - assert next(remotes.get("@repo1")).address == "https://www.example.com" + assert next(remotes.get("@reg2")).address == "oci://www.example.com/subfolder" with pytest.raises(HelmRemoteAliasNotFoundError): list(remotes.get("@reg3")) @@ -34,23 +33,18 @@ def test_helm_remotes() -> None: assert list(remotes.get("oci://www.example.com/charts")) == [ HelmRegistry(address="oci://www.example.com/charts") ] - assert list(remotes.get("https://www.example.com/charts")) == [ - HelmClassicRepository(address="https://www.example.com/charts") - ] # Test defaults. - remotes = HelmRemotes.from_dicts( + remotes = HelmRemotes.from_dict( { "default": {"address": "oci://www.example.com/default"}, "reg1": {"address": "oci://www.example.com/charts1", "default": "false"}, "reg2": {"address": "oci://www.example.com/charts2", "default": "true"}, "reg3": {"address": "oci://www.example.com/charts3", "default": "true"}, }, - {}, ) - print(remotes.default) - assert cast(HelmRegistry, next(remotes.get("@reg2"))).default is True + assert next(remotes.get("@reg2")).default is True assert [r.address for r in remotes.default] == [ "oci://www.example.com/charts2", "oci://www.example.com/charts3", diff --git a/src/python/pants/backend/helm/subsystems/helm.py b/src/python/pants/backend/helm/subsystems/helm.py index 6eb8952ced5..6672a1630f3 100644 --- a/src/python/pants/backend/helm/subsystems/helm.py +++ b/src/python/pants/backend/helm/subsystems/helm.py @@ -7,12 +7,7 @@ from typing import Any from pants.backend.helm.resolve.remotes import HelmRemotes -from pants.backend.helm.target_types import ( - HelmArtifactRepositoryField, - HelmArtifactTarget, - HelmChartTarget, - HelmRegistriesField, -) +from pants.backend.helm.target_types import HelmChartTarget, HelmRegistriesField from pants.core.util_rules.external_tool import TemplatedExternalTool from pants.engine.platform import Platform from pants.option.option_types import BoolOption, DictOption @@ -44,26 +39,6 @@ """ ) -classic_repositories_help = softwrap( - f""" - Configure Helm Classic repositories. The schema for a registry entry is as follows: - - {{ - "repository-alias": {{ - "address": "http://repository-domain:port", - }}, - ... - }} - - Classic repositories are used to provide support for Helm third party charts available at Chart Museum - or similar places. - - To be able to reference third party charts in classic repos, you need to assign an alias to each of them - as per the previous schema and then declare a `{HelmArtifactTarget.alias}` target and set its - `{HelmArtifactRepositoryField.alias}` to the given alias of the classic repository prefixed by a `@`. - """ -) - class HelmSubsystem(TemplatedExternalTool): options_scope = "helm" @@ -85,9 +60,6 @@ class HelmSubsystem(TemplatedExternalTool): } _registries = DictOption[Any]("--registries", help=registries_help, fromfile=True) - _classic_repositories = DictOption[Any]( - "--classic-repositories", help=classic_repositories_help, fromfile=True - ) lint_strict = BoolOption( "--lint-strict", default=False, help="Enables strict linting of Helm charts" ) @@ -99,4 +71,4 @@ def generate_exe(self, plat: Platform) -> str: @memoized_method def remotes(self) -> HelmRemotes: - return HelmRemotes.from_dicts(self._registries, self._classic_repositories) + return HelmRemotes.from_dict(self._registries) diff --git a/src/python/pants/backend/helm/target_types.py b/src/python/pants/backend/helm/target_types.py index 8ce207d3063..8830d04e56f 100644 --- a/src/python/pants/backend/helm/target_types.py +++ b/src/python/pants/backend/helm/target_types.py @@ -228,14 +228,22 @@ class HelmUnitTestTestsGeneratorTarget(TargetFilesGenerator): class HelmArtifactRegistryField(StringField): alias = "registry" - help = ( - "Registry alias (prefixed by `@`) configured in `[helm.registries]` for the Helm artifact." + help = softwrap( + """ + Either registry alias (prefixed by `@`) configured in `[helm.registries]` for the + Helm artifact or the full OCI registry URL. + """ ) class HelmArtifactRepositoryField(StringField): alias = "repository" - help = "Either an alias (prefixed by `@`) to a classic Helm repository configured in `[helm.registries]` or a path inside an OCI registry." + help = softwrap( + f""" + Either a HTTP(S) URL to a classic repository, or a path inside an OCI registry (when + `{HelmArtifactRegistryField.alias}` is provided). + """ + ) class HelmArtifactArtifactField(StringField): diff --git a/src/python/pants/backend/helm/util_rules/chart.py b/src/python/pants/backend/helm/util_rules/chart.py index 1f721656252..42efd01155b 100644 --- a/src/python/pants/backend/helm/util_rules/chart.py +++ b/src/python/pants/backend/helm/util_rules/chart.py @@ -8,6 +8,7 @@ from dataclasses import dataclass from pants.backend.helm.resolve import fetch +from pants.backend.helm.resolve.artifacts import ResolvedHelmArtifact from pants.backend.helm.resolve.fetch import ( FetchedHelmArtifact, FetchedHelmArtifacts, @@ -18,6 +19,7 @@ from pants.backend.helm.util_rules import chart_metadata, sources from pants.backend.helm.util_rules.chart_metadata import ( HELM_CHART_METADATA_FILENAMES, + HelmChartDependency, HelmChartMetadata, ParseHelmChartMetadataDigest, ) @@ -35,6 +37,7 @@ from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.engine.target import DependenciesRequest, Target, Targets from pants.util.logging import LogLevel +from pants.util.ordered_set import OrderedSet from pants.util.strutil import pluralize logger = logging.getLogger(__name__) @@ -45,6 +48,7 @@ class HelmChart: address: Address metadata: HelmChartMetadata snapshot: Snapshot + artifact: ResolvedHelmArtifact | None = None @property def path(self) -> str: @@ -70,7 +74,12 @@ async def create_chart_from_artifact(fetched_artifact: FetchedHelmArtifact) -> H prefix=fetched_artifact.artifact.name, ), ) - return HelmChart(fetched_artifact.address, metadata, fetched_artifact.snapshot) + return HelmChart( + fetched_artifact.address, + metadata, + fetched_artifact.snapshot, + artifact=fetched_artifact.artifact, + ) @rule(desc="Collect all source code and subcharts of a Helm Chart", level=LogLevel.DEBUG) @@ -122,7 +131,7 @@ async def get_helm_chart(request: HelmChartRequest, subsystem: HelmSubsystem) -> # Update subchart dependencies in the metadata and re-render it. remotes = subsystem.remotes() subchart_map: dict[str, HelmChart] = {chart.metadata.name: chart for chart in subcharts} - updated_dependencies = [] + updated_dependencies: OrderedSet[HelmChartDependency] = OrderedSet() for dep in metadata.dependencies: updated_dep = dep @@ -139,7 +148,25 @@ async def get_helm_chart(request: HelmChartRequest, subsystem: HelmSubsystem) -> updated_dep, version=subchart_map[dep.name].metadata.version ) - updated_dependencies.append(updated_dep) + updated_dependencies.add(updated_dep) + + # Include the explicitly provided subchats in the set of dependencies if not already present. + updated_dependencies_names = {dep.name for dep in updated_dependencies} + remaining_subcharts = [ + chart for chart in subcharts if chart.metadata.name not in updated_dependencies_names + ] + for chart in remaining_subcharts: + if chart.artifact: + dependency = HelmChartDependency( + name=chart.artifact.name, + version=chart.artifact.version, + repository=chart.artifact.location_url, + ) + else: + dependency = HelmChartDependency( + name=chart.metadata.name, version=chart.metadata.version + ) + updated_dependencies.add(dependency) # Update metadata with the information about charts' dependencies. metadata = dataclasses.replace(metadata, dependencies=tuple(updated_dependencies)) @@ -164,11 +191,7 @@ async def get_helm_chart(request: HelmChartRequest, subsystem: HelmSubsystem) -> ) chart_snapshot = await Get(Snapshot, AddPrefix(content_digest, metadata.name)) - return HelmChart( - address=request.field_set.address, - metadata=metadata, - snapshot=chart_snapshot, - ) + return HelmChart(address=request.field_set.address, metadata=metadata, snapshot=chart_snapshot) def rules(): diff --git a/src/python/pants/backend/helm/util_rules/chart_metadata.py b/src/python/pants/backend/helm/util_rules/chart_metadata.py index 076e1b7aff8..684320132d0 100644 --- a/src/python/pants/backend/helm/util_rules/chart_metadata.py +++ b/src/python/pants/backend/helm/util_rules/chart_metadata.py @@ -11,7 +11,6 @@ import yaml -from pants.backend.helm.resolve.remotes import OCI_REGISTRY_PROTOCOL, HelmRemotes from pants.backend.helm.target_types import HelmChartMetaSourceField from pants.backend.helm.util_rules.yaml_utils import snake_case_attr_dict from pants.base.glob_match_error_behavior import GlobMatchErrorBehavior @@ -87,18 +86,6 @@ def to_json_dict(self) -> dict[str, Any]: d["import-values"] = list(self.import_values) return d - def remote_spec(self, remotes: HelmRemotes) -> str: - if not self.repository: - registry = remotes.default_registry - if registry: - return f"{registry.address}/{self.name}" - return self.name - elif self.repository.startswith(OCI_REGISTRY_PROTOCOL): - return f"{self.repository}/{self.name}" - else: - remote = remotes.all[self.repository] - return f"{remote.alias}/{self.name}" - @dataclass(frozen=True) class HelmChartMaintainer: diff --git a/src/python/pants/backend/helm/util_rules/chart_test.py b/src/python/pants/backend/helm/util_rules/chart_test.py index b3c5a9fccf7..cf1b6fde884 100644 --- a/src/python/pants/backend/helm/util_rules/chart_test.py +++ b/src/python/pants/backend/helm/util_rules/chart_test.py @@ -22,6 +22,7 @@ ChartType, HelmChartDependency, HelmChartMetadata, + ParseHelmChartMetadataDigest, ) from pants.build_graph.address import Address from pants.core.util_rules import config_files, external_tool, stripped_source_files @@ -45,6 +46,7 @@ def rule_runner() -> RuleRunner: *stripped_source_files.rules(), *target_types_rules(), QueryRule(HelmChart, (HelmChartRequest,)), + QueryRule(HelmChartMetadata, (ParseHelmChartMetadataDigest,)), ], ) @@ -84,6 +86,7 @@ def test_collects_single_chart_sources( ) helm_chart = rule_runner.request(HelmChart, [HelmChartRequest.from_target(tgt)]) + assert not helm_chart.artifact assert helm_chart.metadata == expected_metadata assert len(helm_chart.snapshot.files) == 4 assert helm_chart.address == address @@ -111,6 +114,7 @@ def test_gathers_local_subchart_sources_using_explicit_dependency(rule_runner: R version: 0.1.0 dependencies: - name: chart1 + alias: foo """ ), } @@ -124,6 +128,9 @@ def test_gathers_local_subchart_sources_using_explicit_dependency(rule_runner: R assert "chart2/charts/chart1" in helm_chart.snapshot.dirs assert "chart2/charts/chart1/templates/service.yaml" in helm_chart.snapshot.files + assert len(helm_chart.metadata.dependencies) == 1 + assert helm_chart.metadata.dependencies[0].name == "chart1" + assert helm_chart.metadata.dependencies[0].alias == "foo" def test_gathers_all_subchart_sources_inferring_dependencies(rule_runner: RuleRunner) -> None: @@ -133,7 +140,7 @@ def test_gathers_all_subchart_sources_inferring_dependencies(rule_runner: RuleRu """\ helm_artifact( name="cert-manager", - repository="@jetstack", + repository="https://charts.jetstack.io", artifact="cert-manager", version="v0.7.0" ) @@ -160,18 +167,16 @@ def test_gathers_all_subchart_sources_inferring_dependencies(rule_runner: RuleRu - name: chart1 alias: dep1 - name: cert-manager - repository: "@jetstack" + repository: "https://charts.jetstack.io" """ ), } ) 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}", ] ) @@ -199,3 +204,78 @@ def test_gathers_all_subchart_sources_inferring_dependencies(rule_runner: RuleRu assert "chart2/charts/chart1/templates/service.yaml" in helm_chart.snapshot.files assert "chart2/charts/cert-manager" in helm_chart.snapshot.dirs assert "chart2/charts/cert-manager/Chart.yaml" in helm_chart.snapshot.files + + +def test_chart_metadata_is_updated_with_explicit_dependencies(rule_runner: RuleRunner) -> None: + rule_runner.write_files( + { + "3rdparty/helm/jetstack/BUILD": dedent( + """\ + helm_artifact( + name="cert-manager", + repository="https://charts.jetstack.io", + artifact="cert-manager", + version="v0.7.0" + ) + """ + ), + "src/chart1/BUILD": "helm_chart()", + "src/chart1/Chart.yaml": dedent( + """\ + apiVersion: v2 + name: chart1 + version: 0.1.0 + """ + ), + "src/chart2/BUILD": dedent( + """\ + helm_chart(dependencies=["//src/chart1", "//3rdparty/helm/jetstack:cert-manager"]) + """ + ), + "src/chart2/Chart.yaml": dedent( + """\ + apiVersion: v2 + name: chart2 + version: 0.1.0 + """ + ), + } + ) + + source_root_patterns = ("/src/*",) + rule_runner.set_options( + [ + f"--source-root-patterns={repr(source_root_patterns)}", + ] + ) + + expected_metadata = HelmChartMetadata( + name="chart2", + api_version="v2", + version="0.1.0", + dependencies=( + HelmChartDependency( + name="chart1", + version="0.1.0", + ), + HelmChartDependency( + name="cert-manager", version="v0.7.0", repository="https://charts.jetstack.io" + ), + ), + ) + + target = rule_runner.get_target(Address("src/chart2", target_name="chart2")) + helm_chart = rule_runner.request(HelmChart, [HelmChartRequest.from_target(target)]) + new_metadata = rule_runner.request( + HelmChartMetadata, + [ + ParseHelmChartMetadataDigest( + helm_chart.snapshot.digest, + description_of_origin="test_chart_metadata_is_updated_with_explicit_dependencies", + prefix=helm_chart.path, + ) + ], + ) + + assert helm_chart.metadata == expected_metadata + assert new_metadata == expected_metadata diff --git a/src/python/pants/backend/helm/util_rules/tool.py b/src/python/pants/backend/helm/util_rules/tool.py index 987f33ce57f..34e6100bf97 100644 --- a/src/python/pants/backend/helm/util_rules/tool.py +++ b/src/python/pants/backend/helm/util_rules/tool.py @@ -16,12 +16,17 @@ from pants.engine.fs import CreateDigest, Digest, DigestSubset, Directory, PathGlobs, RemovePrefix from pants.engine.internals.native_engine import AddPrefix, MergeDigests from pants.engine.platform import Platform -from pants.engine.process import Process, ProcessCacheScope, ProcessResult +from pants.engine.process import Process, ProcessCacheScope from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.util.frozendict import FrozenDict from pants.util.logging import LogLevel from pants.util.meta import frozen_after_init +_HELM_CACHE_NAME = "helm" +_HELM_CACHE_DIR = "__cache" +_HELM_CONFIG_DIR = "__config" +_HELM_DATA_DIR = "__data" + @frozen_after_init @dataclass(unsafe_hash=True) @@ -43,6 +48,18 @@ def __init__( self.immutable_input_digests = FrozenDict(immutable_input_digests) self.env = FrozenDict({**helm_env, **local_env}) + @property + def config_digest(self) -> Digest: + return self.immutable_input_digests[_HELM_CONFIG_DIR] + + @property + def data_digest(self) -> Digest: + return self.immutable_input_digests[_HELM_DATA_DIR] + + @property + def append_only_caches(self) -> dict[str, str]: + return {_HELM_CACHE_NAME: _HELM_CACHE_DIR} + @frozen_after_init @dataclass(unsafe_hash=True) @@ -83,10 +100,6 @@ def __init__( @rule(desc="Download and configure Helm", level=LogLevel.DEBUG) async def setup_helm(helm_subsytem: HelmSubsystem, global_plugins: HelmPlugins) -> HelmBinary: - cache_dir = "__cache" - config_dir = "__config" - data_dir = "__data" - downloaded_binary, empty_dirs_digest = await MultiGet( Get( DownloadedExternalTool, ExternalToolRequest, helm_subsytem.get_request(Platform.current) @@ -95,9 +108,8 @@ async def setup_helm(helm_subsytem: HelmSubsystem, global_plugins: HelmPlugins) Digest, CreateDigest( [ - Directory(cache_dir), - Directory(config_dir), - Directory(data_dir), + Directory(_HELM_CONFIG_DIR), + Directory(_HELM_DATA_DIR), ] ), ), @@ -108,81 +120,46 @@ async def setup_helm(helm_subsytem: HelmSubsystem, global_plugins: HelmPlugins) helm_path = os.path.join(tool_relpath, downloaded_binary.exe) helm_env = { - "HELM_CACHE_HOME": cache_dir, - "HELM_CONFIG_HOME": config_dir, - "HELM_DATA_HOME": data_dir, + "HELM_CACHE_HOME": _HELM_CACHE_DIR, + "HELM_CONFIG_HOME": _HELM_CONFIG_DIR, + "HELM_DATA_HOME": _HELM_DATA_DIR, } # Create a digest that will get mutated during the setup process mutable_input_digest = empty_dirs_digest - output_dirs = (cache_dir, config_dir, data_dir) - - def create_process(args: list[str], *, description: str, input_digest: Digest) -> Process: - return Process( - [helm_path, *args], - env=helm_env, - input_digest=input_digest, - immutable_input_digests=immutable_input_digests, - output_directories=output_dirs, - description=description, - level=LogLevel.DEBUG, - ) - - # Configures Helm classic repositories - helm_remotes = helm_subsytem.remotes() - classic_repos = helm_remotes.classic_repositories() - if classic_repos: - for repo in classic_repos: - args = ["repo", "add", repo.alias, repo.address] - result = await Get( - ProcessResult, - Process, - create_process( - args, - description=f"Adding Helm classic repository '{repo.alias}' at: {repo.address}", - input_digest=mutable_input_digest, - ), - ) - mutable_input_digest = result.output_digest - - update_index_result = await Get( - ProcessResult, - Process, - create_process( - ["repo", "update"], - description="Update Helm classic repository indexes", - input_digest=mutable_input_digest, - ), - ) - mutable_input_digest = update_index_result.output_digest - # Install all global Helm plugins if global_plugins: prefixed_plugins_digests = await MultiGet( - Get(Digest, AddPrefix(plugin.digest, os.path.join(data_dir, "plugins", plugin.name))) + Get( + Digest, + AddPrefix(plugin.digest, os.path.join(_HELM_DATA_DIR, "plugins", plugin.name)), + ) for plugin in global_plugins ) mutable_input_digest = await Get( Digest, MergeDigests([mutable_input_digest, *prefixed_plugins_digests]) ) - updated_cache_digest, updated_config_digest, updated_data_digest = await MultiGet( - Get(Digest, DigestSubset(mutable_input_digest, PathGlobs([f"{cache_dir}/**"]))), - Get(Digest, DigestSubset(mutable_input_digest, PathGlobs([f"{config_dir}/**"]))), - Get(Digest, DigestSubset(mutable_input_digest, PathGlobs([f"{data_dir}/**"]))), + updated_config_digest, updated_data_digest = await MultiGet( + Get( + Digest, + DigestSubset(mutable_input_digest, PathGlobs([os.path.join(_HELM_CONFIG_DIR, "**")])), + ), + Get( + Digest, + DigestSubset(mutable_input_digest, PathGlobs([os.path.join(_HELM_DATA_DIR, "**")])), + ), ) - cache_subset_digest, config_subset_digest, data_subset_digest = await MultiGet( - Get(Digest, RemovePrefix(updated_cache_digest, cache_dir)), - Get(Digest, RemovePrefix(updated_config_digest, config_dir)), - Get(Digest, RemovePrefix(updated_data_digest, data_dir)), + config_subset_digest, data_subset_digest = await MultiGet( + Get(Digest, RemovePrefix(updated_config_digest, _HELM_CONFIG_DIR)), + Get(Digest, RemovePrefix(updated_data_digest, _HELM_DATA_DIR)), ) setup_immutable_digests = { **immutable_input_digests, - config_dir: config_subset_digest, - cache_dir: cache_subset_digest, - data_dir: data_subset_digest, + _HELM_CONFIG_DIR: config_subset_digest, + _HELM_DATA_DIR: data_subset_digest, } local_env = await Get(Environment, EnvironmentRequest(["HOME", "PATH"])) @@ -210,6 +187,7 @@ def helm_process(request: HelmProcess, helm_binary: HelmBinary) -> Process: env=env, description=request.description, level=request.level, + append_only_caches=helm_binary.append_only_caches, output_directories=request.output_directories, output_files=request.output_files, cache_scope=request.cache_scope or ProcessCacheScope.SUCCESSFUL, diff --git a/src/python/pants/backend/helm/util_rules/tool_test.py b/src/python/pants/backend/helm/util_rules/tool_test.py index d9baf49dfc5..0485a6b952e 100644 --- a/src/python/pants/backend/helm/util_rules/tool_test.py +++ b/src/python/pants/backend/helm/util_rules/tool_test.py @@ -3,8 +3,6 @@ from __future__ import annotations -import re - import pytest from pants.backend.helm.subsystems.helm import HelmSubsystem @@ -40,36 +38,11 @@ def rule_runner() -> RuleRunner: def test_initialises_basic_helm_binary(rule_runner: RuleRunner) -> None: helm_subsystem = rule_runner.request(HelmSubsystem, []) helm_binary = rule_runner.request(HelmBinary, []) + assert helm_binary assert helm_binary.path == f"__helm/{helm_subsystem.generate_exe(Platform.current)}" -def test_initialise_classic_repos(rule_runner: RuleRunner) -> None: - repositories_opts = """{"jetstack": {"address": "https://charts.jetstack.io"}}""" - rule_runner.set_options([f"--helm-classic-repositories={repositories_opts}"]) - - process = HelmProcess( - argv=["repo", "list"], - input_digest=EMPTY_DIGEST, - description="List installed classic repositories", - ) - result = rule_runner.request(ProcessResult, [process]) - - # The result of the `helm repo list` command is a table with a header like - # NAME URL - # alias http://example.com/charts - # - # So to build the test expectation we parse that output keeping - # the repository's alias and url to be used in the comparison - table_rows = result.stdout.decode().splitlines()[1:] - configured_repos = [ - (columns[0].strip(), columns[1].strip()) - for columns in (re.split(r"\t+", line.rstrip()) for line in table_rows) - ] - - assert configured_repos == [("jetstack", "https://charts.jetstack.io")] - - def test_create_helm_process(rule_runner: RuleRunner) -> None: helm_binary = rule_runner.request(HelmBinary, [])