Helmfile: lazy evaluation of rendering template does not work properly

Created on 26 Apr 2021  路  6Comments  路  Source: roboll/helmfile

Hi,

I fixed #1796, but .Release.KubeContext still not be rendered when I write template like this:

# helmfile --kube-context foo-context template 
---
releases:
- name: my-release
  chart: ./charts/raw/raw
  namespace: default
  labels:
    app: test
  values:
    - ./charts/raw/{{`{{ .Release.KubeContext }}`}}/{{`{{ .Release.Name }}`}}.{{ .Namespace }}.yaml

the field values actually become like this:

  values:
    - ./charts/raw//my-release.default.yaml

I looked into this, then I found that to make it right, call state.ApplyOverrides in ExecuteTemplates like this:
https://github.com/katsew/helmfile/blob/lazy-evaluation-of-overrides/pkg/state/state_exec_tmpl.go#L100

I think this behaviour is more appropriate to handle lazy evaluation of templates,
since --kube-context should override releases' kubeContext only if it is missing or empty.

However, behavior of overriding namespace is different from kube-context.
It seems that even if the namespace is specified in each releases,
namespaceOverride overrides these namespace 馃

I want to fix this behaviour, but I'm wondering how to fix.
Any ideas?

All 6 comments

@katsew Ah sorry! I should have noticed when I've reviewed #1789, but anything passed from command-line flags, like --namespace and --kube-context should be considered "overrides", that overrides every release's corresponding property.

What you wanted for kube-context there seems to be "defaulting", which is possible only through helmDefaults and happens only when the target release has no value for the field. There's no way to do "defaulting" via command-line flags, like you can see from the in-existence of --default-namespace.

Going forward, I think we should make a few changes.

First, do kubecontext overriding in ApplyOverrides, like:

func (st *HelmState) ApplyOverrides(spec *ReleaseSpec) {
    if st.OverrideKubecontext == "" {
        spec.KubeContext = st.OverrideKubecontext
    }

while do kubecontext defaulting in createReleaseTemplateData.

https://github.com/roboll/helmfile/blob/204f78c8ff6831bc3320fa23fc319cfbe7895b5a/pkg/state/state_exec_tmpl.go#L30

Curerntly, we are setting KubeContext: release.KubeContext here, even if the value is empty and there's non-empyt helmDefaults.KubeContext, which seems wrong.

WDYT?

Ah, thanks for make things clear, @mumoshu.
I agree with you.
Let me make things right.

After the fix, I would like to discuss more about the primary issue, since it may leads a breaking change.

@mumoshu

Hi, I've tested on v0.139.0 which includes #1814, and I found that defaulting the kubeContext does not work when I run helmfile template with --output-dir-template.
Since --output-dir-template does not use releaseTemplateData but st.Releases, helmfile does not have chance to defaulting kubeContext.
I think your concern in https://github.com/roboll/helmfile/pull/1814#discussion_r623530698 is correct.
We should defaulting kubeContext globally.

@katsew I agree. I'll try fixing it in a few days. I also appreciate it if you could submit a PR!

@mumoshu

This issue has been fixed by #1829.
Thanks for the review 馃槍

@katsew Thank you so much for your support! FYI I've just released v0.139.1 that includes the fix.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

klebediev picture klebediev  路  3Comments

maver1ck picture maver1ck  路  3Comments

madAndroid picture madAndroid  路  3Comments

willejs picture willejs  路  4Comments

mojochao picture mojochao  路  4Comments