Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Values merge with nil and map doesn't work #1150

Closed
mattymo opened this issue Mar 19, 2020 · 4 comments · Fixed by #1153
Closed

Values merge with nil and map doesn't work #1150

mattymo opened this issue Mar 19, 2020 · 4 comments · Fixed by #1153

Comments

@mattymo
Copy link

mattymo commented Mar 19, 2020

Problem: Map should always win over nil in a values merge scenario.

Scenario to reproduce

values1.yaml:

components:
  etcd-operator:

values2.yaml:

components:
  etcd-operator:
    version:  0.10.3

helmfile.yaml

repositories:
- name: "stable"
  url: "https://kubernetes-charts.storage.googleapis.com"

environments:
  default:
    values:
      - values1.yaml
      - values2.yaml

releases:
- name: "etcd-operator"
  chart: stable/etcd-operator
{{- if .Values.components | getOrNil "etcd-operator" | getOrNil "version" }}
  version: {{ .Values.components  | getOrNil "etcd-operator"  | getOrNil "version"  }}
{{- end }}

The result is that the second values file doesn't properly merge if the first one contains any nil values. In the resulting values set, .Values.components["etcd-operator"] is nil.

Helmfile version:
helmfile version v0.102.0

@mumoshu
Copy link
Collaborator

mumoshu commented Mar 20, 2020

@mattymo Good catch! Unfortunately, we can't easily avoid this due to the library which Helmfile uses under the hood to merge maps. The issue is darccio/mergo#123, which prevents Helmfile from overwriting nil values with maps.

The workaround would be to modify your values1.yaml to have an empty object where you expect to overwrite later, so that it looks:

components:
  etcd-operator: {}
$ helmfile build
---
#  Source: helmfile.yaml

filepath: helmfile.yaml
environments:
  default:
    values:
    - values1.yaml
    - values2.yaml
repositories:
- name: stable
  url: https://kubernetes-charts.storage.googleapis.com
releases:
- chart: stable/etcd-operator
  version: 0.10.3
  name: etcd-operator
templates: {}

@mattymo
Copy link
Author

mattymo commented Mar 20, 2020

@mumoshu Thanks for the quick response. I really hope someone fixes it in the library soon. This can't be an impossible task

mumoshu added a commit that referenced this issue Mar 20, 2020
mumoshu added a commit that referenced this issue Mar 20, 2020
@mumoshu
Copy link
Collaborator

mumoshu commented Mar 20, 2020

@mattymo I was reading the mergo code and it turned out that bumping mergo fixes this specific issue for me. Could you give it a try with Helmfile v0.104.0 which has just been released?

@mattymo
Copy link
Author

mattymo commented Mar 23, 2020

@mumoshu I confirmed this bug is fixed. I uncovered yet another (similar) one and I'll open a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants