Skip to content

Commit

Permalink
Make sure FirstPartyHelmDeploymentMapping doesn't get calculated if d…
Browse files Browse the repository at this point in the history
…isabled (Cherry-pick of #21633) (#21635)

Option `[helm-infer].deployment_dependencies` was added in
#21282. Unfortunately, I missed
one place where `FirstPartyHelmDeploymentMapping` is requested:
```python
@rule(desc="Prepare Helm deployment post-renderer", level=LogLevel.DEBUG)
async def prepare_post_renderer_for_helm_deployment(
    request: HelmDeploymentPostRendererRequest,
    union_membership: UnionMembership,
    docker_options: DockerOptions,
) -> SetupHelmPostRenderer:
    mapping = await Get(
        FirstPartyHelmDeploymentMapping, FirstPartyHelmDeploymentMappingRequest(request.field_set)
    )
    ...
```
Here I move this logic inside the rule, so that all consumers correctly
handle the case when `[helm-infer].deployment_dependencies` is set to
`false`

Co-authored-by: Gregory Borodin <[email protected]>
  • Loading branch information
WorkerPants and grihabor authored Nov 12, 2024
1 parent ef585bb commit e0aa197
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 22 deletions.
48 changes: 31 additions & 17 deletions src/python/pants/backend/helm/dependency_inference/deployment.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,32 @@ class FirstPartyHelmDeploymentMapping:
indexed_docker_addresses: FrozenYamlIndex[tuple[str, Address]]


@dataclass(frozen=True)
class _FirstPartyHelmDeploymentMappingRequest(EngineAwareParameter):
field_set: HelmDeploymentFieldSet


@rule
async def first_party_helm_deployment_mapping(
request: FirstPartyHelmDeploymentMappingRequest,
helm_infer: HelmInferSubsystem,
) -> FirstPartyHelmDeploymentMapping:
if not helm_infer.deployment_dependencies:
return FirstPartyHelmDeploymentMapping(
request.field_set.address,
FrozenYamlIndex.empty(),
)
# Use a small proxy rule to make sure we don't calculate AllDockerImageTargets
# if `[helm-infer].deployment_dependencies` is set to true.
return await Get(
FirstPartyHelmDeploymentMapping,
_FirstPartyHelmDeploymentMappingRequest(field_set=request.field_set),
)


@rule
async def _first_party_helm_deployment_mapping(
request: _FirstPartyHelmDeploymentMappingRequest,
docker_targets: AllDockerImageTargets,
helm_infer: HelmInferSubsystem,
) -> FirstPartyHelmDeploymentMapping:
Expand Down Expand Up @@ -286,23 +309,14 @@ async def inject_deployment_dependencies(
DependenciesRequest(request.field_set.dependencies),
)

if infer_subsystem.deployment_dependencies:
chart_address, explicitly_provided_deps, mapping = await MultiGet(
get_address,
get_explicit_deps,
Get(
FirstPartyHelmDeploymentMapping,
FirstPartyHelmDeploymentMappingRequest(request.field_set),
),
)
else:
(chart_address, explicitly_provided_deps), mapping = (
await MultiGet(get_address, get_explicit_deps),
FirstPartyHelmDeploymentMapping(
request.field_set.address,
FrozenYamlIndex.empty(),
),
)
chart_address, explicitly_provided_deps, mapping = await MultiGet(
get_address,
get_explicit_deps,
Get(
FirstPartyHelmDeploymentMapping,
FirstPartyHelmDeploymentMappingRequest(request.field_set),
),
)

dependencies: OrderedSet[Address] = OrderedSet()
dependencies.add(chart_address)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,10 @@ def rule_runner() -> RuleRunner:
*process.rules(),
*stripped_source_files.rules(),
*tool.rules(),
QueryRule(FirstPartyHelmDeploymentMapping, (FirstPartyHelmDeploymentMappingRequest,)),
QueryRule(
FirstPartyHelmDeploymentMapping,
(FirstPartyHelmDeploymentMappingRequest,),
),
QueryRule(HelmDeploymentReport, (AnalyseHelmDeploymentRequest,)),
QueryRule(InferredDependencies, (InferHelmDeploymentDependenciesRequest,)),
],
Expand Down Expand Up @@ -102,7 +105,8 @@ def test_deployment_dependencies_report(rule_runner: RuleRunner) -> None:

source_root_patterns = ("/src/*",)
rule_runner.set_options(
[f"--source-root-patterns={repr(source_root_patterns)}"], env_inherit=PYTHON_BOOTSTRAP_ENV
[f"--source-root-patterns={repr(source_root_patterns)}"],
env_inherit=PYTHON_BOOTSTRAP_ENV,
)

target = rule_runner.get_target(Address("src/deployment", target_name="foo"))
Expand Down Expand Up @@ -230,13 +234,17 @@ def test_resolve_relative_docker_addresses_to_deployment(

def make_request():
return rule_runner.request(
FirstPartyHelmDeploymentMapping, [FirstPartyHelmDeploymentMappingRequest(field_set)]
FirstPartyHelmDeploymentMapping,
[FirstPartyHelmDeploymentMappingRequest(field_set)],
)

if correct_target_name:
expected = [
(":myapp0", Address("src/deployment", target_name="myapp0")),
("//src/deployment:myapp1", Address("src/deployment", target_name="myapp1")),
(
"//src/deployment:myapp1",
Address("src/deployment", target_name="myapp1"),
),
("./subdir:myapp2", Address("src/deployment/subdir", target_name="myapp2")),
]
assert list(make_request().indexed_docker_addresses.values()) == expected
Expand Down Expand Up @@ -377,7 +385,8 @@ def test_inject_deployment_dependencies(rule_runner: RuleRunner) -> None:
expected_dependency_addr = Address("src/image", target_name="myapp")

mapping = rule_runner.request(
FirstPartyHelmDeploymentMapping, [FirstPartyHelmDeploymentMappingRequest(field_set)]
FirstPartyHelmDeploymentMapping,
[FirstPartyHelmDeploymentMappingRequest(field_set)],
)
assert list(mapping.indexed_docker_addresses.values()) == [
(expected_image_ref, expected_dependency_addr)
Expand Down Expand Up @@ -447,3 +456,60 @@ def test_disambiguate_docker_dependency(rule_runner: RuleRunner) -> None:
# Assert only the Helm chart dependency has been inferred
assert len(inferred_dependencies.include) == 1
assert set(inferred_dependencies.include) == {Address("src/mychart")}


def test_dont_infer_docker_dependency(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
"src/mychart/BUILD": "helm_chart()",
"src/mychart/Chart.yaml": HELM_CHART_FILE,
"src/mychart/values.yaml": HELM_VALUES_FILE,
"src/mychart/templates/_helpers.tpl": HELM_TEMPLATE_HELPERS_FILE,
"src/mychart/templates/pod.yaml": dedent(
"""\
apiVersion: v1
kind: Pod
metadata:
name: {{ template "fullname" . }}
labels:
chart: "{{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}"
spec:
containers:
- name: myapp-container
image: registry/image:latest
"""
),
"src/deployment/BUILD": dedent(
"""\
helm_deployment(
name="foo",
chart="//src/mychart",
)
"""
),
"src/docker/BUILD": "docker_image(name='latest')",
"src/docker/Dockerfile": "FROM busybox:1.28",
}
)

source_root_patterns = ("/", "src/*")
rule_runner.set_options(
[
f"--source-root-patterns={repr(source_root_patterns)}",
"--no-helm-infer-deployment-dependencies",
"--helm-infer-unowned-dependency-behavior=error",
],
env_inherit=PYTHON_BOOTSTRAP_ENV,
)

deployment_addr = Address("src/deployment", target_name="foo")
tgt = rule_runner.get_target(deployment_addr)

inferred_dependencies = rule_runner.request(
InferredDependencies,
[InferHelmDeploymentDependenciesRequest(HelmDeploymentFieldSet.create(tgt))],
)

# Assert only the Helm chart dependency has been inferred
assert len(inferred_dependencies.include) == 1
assert set(inferred_dependencies.include) == {Address("src/mychart")}

0 comments on commit e0aa197

Please sign in to comment.