This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Nice ! 💪 @mumoshu , with the additional layeredValues flag to guard against these changes being breaking,
do you think we could potentially include this before moving to a major version ? 🙂

I like this implementation as it looks like a fairly simple way to reduce boilerplate while avoiding exec & loadYAML machinery 🙏

@Vince-Chenal @hileef Hey! First of all, this looks awesome! I'm definitely looking forward to merging this with only a few changes described below. Can you confirm?

My motivation for doing that is to make it work in concert with environmentTemplates and inherit feature proposed and discussed in #840 (comment).
Could you two read that thread, and my combined proposal below, and mind giving me feedbacks if any? 🙏

So what I'd like to do before merging this pull request is to change layedValues from a flag to another array of values.yaml files that can co-exist with the existing values so that it plays nicely with the environmentTemplates.

environmentTemplates:
  base:
    values:
    - common-static-values.yaml
    layeredValues:
    - common-dynamic-values-1.yaml
    - common-dynamic-values-2.yaml

environments:
  test:
    inherit:
      template: base
    values:
    - adhoc-static-values.yaml
    layeredValues:
    - adhoc-dynamic-values-1.yaml
    - adhoc-dynamic-values-2.yaml

This will result in the test environment to be:

environments:
  test:
    values:
    # (1) sees empty Values when rendering itself
    - common-static-values.yaml
    # (2) sees empty Values when rendering itself
    - adhoc-static-values.yaml
    layeredValues:
    # (3) sees (1)+(2)
    - common-dynamic-values-1.yaml
    # (4) sees (1)+(2)+(3)
    - common-dynamic-values-2.yaml
    # (5) sees (1)+(2)+(3)+(4)
    - adhoc-dynamic-values-1.yaml
    # (6) sees (1)+(2)+(3)+(4)+(5)
    - adhoc-dynamic-values-2.yaml

This way, we can inject values "before the common layedValues derived from the template, but after all static values" via adhoc-static-values.yaml. In other words, you can now pass inputs to the template-declared adhoc-dynamic-values-*.yaml files.

Not only each layedValues entry can see the latest merged values, the first layeredValues entry should see the values from common-static-values.yaml and adhoc-static-values.yaml for the input passing use-case I've described in the previous paragraph.

All in all, this design would give you all the benefits of environment templates along with layered values. Maximum reusability and DRY. WDYT?

Hello @mumoshu , thanks for the feedback !

I've had a look at #840 and your summary proposal here, IMHO I think it's great 💪
I like that we can maintain the requested features here in a terse way,
while opening the door for supporting even more use cases ;

I'll open a PR targetting this one shortly to add the change you requested,
I'm pretty sure that @Vince-Chenal will be ok with approving it 🙂
Once done, we can probably re-update this PR and request your review for merge once again 🙏

Hello again @mumoshu , it took me a while to have time to spend on this topic,
but we've now updated this PR to propose an implementation for the design as proposed in your above comment 🙂

➡️ could you take a look and let us know if these updated changes fits your expectation ?
cc. @Vince-Chenal keeping you in the loop

Hello @mumoshu , @yxxhero 👋
We've updated this PR to resolve the remaining conflicts against the main branch just now 🙏

It seems that you haven't had a chance to take a look at its contents yet,
could you let us know if there are things we can do in the meantime to help move this forward ? 🙂

Hey everyone! OK then... can we make this a per-helmfile.yaml setting, rather than a runtime flag?

Just a quick guess, but having something like:

globalFeatures:
- layeredEnvironmentValues

in helmfile.yaml would work 🤔

It shouldn't be simple features a.k.a local features because this flag will "persist" down to all the sub-helmfiles processed along the way in its current form, an environment variable.

Or, you don't mind setting this feature flag for every helmfile.yaml you use, naming it like features: would be more appropriate, because it's now a per-helmfile.yaml declarative config.

features:
- layeredEnvironmentValues

It may NOT sound like a backward-compatibility breaker to you, because it breaks only when you enabled this runtime feature, yeah...

However, regardless of how it is called, I'm concerned about the possibility that the same helmfile.yaml can be translated differently depending on a runtime setting of Helmfile, which eliminates the benefit of Helmfile- declarativeness. To retain declarativeness as we enhance our configuration language, built around YAML and Go templates, I'd still say that having a declarative way to change Helmfile's behavior is key.

Thank you very much for your patience @Vince-Chenal, and everyone commented on this long-standing proposal!

I hope you are still around, and my suggestion makes sense to you.

I'll also ping @yxxhero and @zhaque44 on our community Slack too for visibility 👋

Hey @mumoshu,
No problem for me about enabling this behavior explicitly.
I just pushed a new version with a features field at helmfile root that allows to enable this feature explicitly by passing layeredEnvironmentValues. Maybe I'm missing some implications with sub-helmfiles or else?

Tell me if that's what you had in mind

closing this, it is outdated at this point

@zhaque44 How is this outdated? Has there been a recent build that enables an equivalent feature?