Skip to content

Commit

Permalink
Add deep-merge of values, and allow empty values file (#812)
Browse files Browse the repository at this point in the history
Allow empty YAML values files.
Perform a deep merge instead of shallow, for better helm values
patching.

(I was unable to locally run the tests, would like to check via the
pipeline)

---------

Co-authored-by: Jan Buijnsters <[email protected]>
Co-authored-by: Allen Porter <[email protected]>
  • Loading branch information
3 people authored Dec 23, 2024
1 parent 59b7cea commit e429a37
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 5 deletions.
23 changes: 21 additions & 2 deletions flux_local/values.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,22 @@ def _lookup_value_reference(
return found_value


def _deep_merge(base: dict[str, Any], override: dict[str, Any]) -> dict[str, Any]:
"""Recursively merge two dictionaries, similar to how Helm merges values. Lists are replaced entirely (Helm behavior)."""
result = base.copy()
for key, override_value in override.items():
base_value = result.get(key)
if (
base_value is not None
and isinstance(base_value, dict)
and isinstance(override_value, dict)
):
result[key] = _deep_merge(base_value, override_value)
else:
result[key] = override_value
return result


def _update_helmrelease_values(
ref: ValuesReference,
found_value: str,
Expand All @@ -196,11 +212,14 @@ def _update_helmrelease_values(
inner_values[parts[-1]] = found_value
else:
obj = yaml.load(found_value, Loader=yaml.SafeLoader)
if not obj or not isinstance(obj, dict):
# Handle empty YAML file case
if obj is None:
obj = {}
if not isinstance(obj, dict):
raise InputException(
f"Expected '{ref.name}' field '{ref.target_path}' values to be valid yaml, found {type(values)}"
)
values.update(obj)
values = _deep_merge(values, obj)

return values

Expand Down
6 changes: 5 additions & 1 deletion tests/testdata/cluster8/apps/podinfo-values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ data:
redis:
enabled: true
repository: public.ecr.aws/docker/library/redis
tag: 7.0.6
tag: 7.0.5
ingress:
enabled: true
className: nginx
Expand All @@ -18,3 +18,7 @@ data:
paths:
- path: /
pathType: ImplementationSpecific
empty-values.yaml: ""
patch-values.yaml: |-
redis:
tag: 7.0.6
6 changes: 6 additions & 0 deletions tests/testdata/cluster8/apps/podinfo.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ spec:
valuesFrom:
- kind: ConfigMap
name: podinfo-values
- kind: ConfigMap
name: podinfo-values
valuesKey: empty-values.yaml
- kind: ConfigMap
name: podinfo-values
valuesKey: patch-values.yaml
- kind: Secret
name: podinfo-tls-values
valuesKey: crt
Expand Down
24 changes: 22 additions & 2 deletions tests/tool/__snapshots__/test_build.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,12 @@
valuesFrom:
- kind: ConfigMap
name: podinfo-values
- kind: ConfigMap
name: podinfo-values
valuesKey: empty-values.yaml
- kind: ConfigMap
name: podinfo-values
valuesKey: patch-values.yaml
- kind: Secret
name: podinfo-tls-values
optional: true
Expand All @@ -956,11 +962,15 @@
---
apiVersion: v1
data:
empty-values.yaml: ""
patch-values.yaml: |-
redis:
tag: 7.0.6
values.yaml: |-
redis:
enabled: true
repository: public.ecr.aws/docker/library/redis
tag: 7.0.6
tag: 7.0.5
ingress:
enabled: true
className: nginx
Expand Down Expand Up @@ -6333,6 +6343,12 @@
valuesFrom:
- kind: ConfigMap
name: podinfo-values
- kind: ConfigMap
name: podinfo-values
valuesKey: empty-values.yaml
- kind: ConfigMap
name: podinfo-values
valuesKey: patch-values.yaml
- kind: Secret
name: podinfo-tls-values
optional: true
Expand All @@ -6341,11 +6357,15 @@
---
apiVersion: v1
data:
empty-values.yaml: ""
patch-values.yaml: |-
redis:
tag: 7.0.6
values.yaml: |-
redis:
enabled: true
repository: public.ecr.aws/docker/library/redis
tag: 7.0.6
tag: 7.0.5
ingress:
enabled: true
className: nginx
Expand Down

0 comments on commit e429a37

Please sign in to comment.