Skip to content

Commit

Permalink
Fix merging of values to override inline values last (#815)
Browse files Browse the repository at this point in the history
https://fluxcd.io/flux/components/helm/helmreleases/#values-references
specifies that `values` are merged last after any `valuesFrom`
references.

Fixes #813
  • Loading branch information
allenporter authored Dec 26, 2024
1 parent e4a668c commit e3f6de8
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 9 deletions.
7 changes: 5 additions & 2 deletions flux_local/values.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ def _lookup_value_reference(

elif (found_value := found_data.get(ref.values_key)) is None:
raise InvalidValuesReference(
"Unable to find key {ref.values_key} in {namespace}/{ref.name}"
f"Unable to find key {ref.values_key} in {namespace}/{ref.name}"
)

return found_value
Expand Down Expand Up @@ -230,7 +230,7 @@ def expand_value_references(
if not helm_release.values_from:
return helm_release

values = helm_release.values or {}
values: dict[str, Any] = {}
cluster_config = ks_cluster_config([kustomization])
for ref in helm_release.values_from:
_LOGGER.debug("Expanding value reference %s", ref)
Expand All @@ -257,6 +257,9 @@ def expand_value_references(
f"Error building HelmRelease '{helm_release.namespaced_name}': {err}"
)

if helm_release.values:
values = _deep_merge(values, helm_release.values)

helm_release.values = values
return helm_release

Expand Down
90 changes: 86 additions & 4 deletions tests/test_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import base64
from pathlib import Path
import yaml
from typing import Any

import pytest
from syrupy.assertion import SnapshotAssertion
Expand Down Expand Up @@ -101,6 +103,75 @@ def test_values_references_with_values_key() -> None:
}



@pytest.mark.parametrize(
("inline_values", "expected_values"),
[
(
{
"redis": {
"enabled": True,
}
},
{
"redis": {
"enabled": True,
}
}
),
(
{},
{
"redis": {
"enabled": False,
}
}
)
]
)
def test_value_reference_ordering(inline_values: dict[str, Any], expected_values: dict[str, Any]) -> None:
"""Test for expanding a value reference with inline values that overwrite."""
hr = HelmRelease(
name="test",
namespace="test",
chart=HelmChart(
repo_name="test-repo",
repo_namespace="flux-system",
name="test-chart",
version="test-version",
),
values={
**inline_values,
},
values_from=[
ValuesReference(
kind="ConfigMap",
name="test-binary-data-configmap",
values_key="some-key",
),
],
)
ks = Kustomization(
name="test",
namespace="test",
path="example/path",
helm_releases=[hr],
config_maps=[
ConfigMap(
name="test-binary-data-configmap",
namespace="test",
binary_data={
"some-key": base64.b64encode(
"redis:\n enabled: False".encode("utf-8")
)
},
),
],
)
updated_hr = expand_value_references(hr, ks)
assert updated_hr.values == expected_values


def test_values_references_with_missing_values_key() -> None:
"""Test for expanding a value reference with a values key that is missing."""
hr = HelmRelease(
Expand Down Expand Up @@ -300,11 +371,12 @@ def test_values_reference_invalid_target_path() -> None:
name="test-chart",
version="test-version",
),
values={
"test": "test",
"target": ["a", "b", "c"],
},
values={},
values_from=[
ValuesReference(
kind="ConfigMap",
name="test-values-original-configmap",
),
ValuesReference(
kind="ConfigMap",
name="test-values-configmap",
Expand All @@ -324,6 +396,16 @@ def test_values_reference_invalid_target_path() -> None:
name="test-values-configmap",
namespace="test",
data={"some-key": "example_value"},
),
ConfigMap(
name="test-values-original-configmap",
namespace="test",
data={
"values.yaml": yaml.dump({
"test": "test",
"target": ["a", "b", "c"],
}),
}
)
],
)
Expand Down
1 change: 0 additions & 1 deletion tests/testdata/cluster8/apps/podinfo-values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ metadata:
data:
values.yaml: |-
redis:
enabled: true
repository: public.ecr.aws/docker/library/redis
tag: 7.0.5
ingress:
Expand Down
3 changes: 3 additions & 0 deletions tests/testdata/cluster8/apps/podinfo.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ spec:
install:
remediation:
retries: 3
values:
redis:
enabled: true
valuesFrom:
- kind: ConfigMap
name: podinfo-values
Expand Down
8 changes: 6 additions & 2 deletions tests/tool/__snapshots__/test_build.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -945,6 +945,9 @@
retries: 3
interval: 50m
releaseName: podinfo
values:
redis:
enabled: true
valuesFrom:
- kind: ConfigMap
name: podinfo-values
Expand All @@ -968,7 +971,6 @@
tag: 7.0.6
values.yaml: |-
redis:
enabled: true
repository: public.ecr.aws/docker/library/redis
tag: 7.0.5
ingress:
Expand Down Expand Up @@ -6340,6 +6342,9 @@
retries: 3
interval: 50m
releaseName: podinfo
values:
redis:
enabled: true
valuesFrom:
- kind: ConfigMap
name: podinfo-values
Expand All @@ -6363,7 +6368,6 @@
tag: 7.0.6
values.yaml: |-
redis:
enabled: true
repository: public.ecr.aws/docker/library/redis
tag: 7.0.5
ingress:
Expand Down

0 comments on commit e3f6de8

Please sign in to comment.